Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.
On Thu, Dec 20, 2018 at 8:14 PM Mohammad Izadi wrote: > From: Mohammad Izadi > > The dynamic metadata contains data for color volume transform - > application 4 of SMPTE 2094-40:2016 standard. The data comes from HEVC in > the SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35. > > pushed, thanks -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.
From: Mohammad Izadi
The dynamic metadata contains data for color volume transform - application 4
of SMPTE 2094-40:2016 standard. The data comes from HEVC in the
SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35.
I'll add support to HEVC in a follow-up.
---
libavutil/Makefile | 2 +
libavutil/frame.c| 1 +
libavutil/frame.h| 7 +
libavutil/hdr_dynamic_metadata.c | 47 +
libavutil/hdr_dynamic_metadata.h | 343 +++
libavutil/version.h | 4 +-
6 files changed, 402 insertions(+), 2 deletions(-)
create mode 100644 libavutil/hdr_dynamic_metadata.c
create mode 100644 libavutil/hdr_dynamic_metadata.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index caddc7e155..7dcb92b06b 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -31,6 +31,7 @@ HEADERS = adler32.h
\
file.h\
frame.h \
hash.h\
+ hdr_dynamic_metadata.h\
hmac.h\
hwcontext.h \
hwcontext_cuda.h \
@@ -119,6 +120,7 @@ OBJS = adler32.o
\
fixed_dsp.o \
frame.o \
hash.o \
+ hdr_dynamic_metadata.o \
hmac.o \
hwcontext.o \
imgutils.o \
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 9b3fb13e68..34a6210d9e 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -840,6 +840,7 @@ const char *av_frame_side_data_name(enum
AVFrameSideDataType type)
case AV_FRAME_DATA_QP_TABLE_PROPERTIES: return "QP table
properties";
case AV_FRAME_DATA_QP_TABLE_DATA: return "QP table data";
#endif
+case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata
SMPTE2094-40 (HDR10+)";
}
return NULL;
}
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 66f27f44bd..582ac470b2 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -166,6 +166,13 @@ enum AVFrameSideDataType {
* function in libavutil/timecode.c.
*/
AV_FRAME_DATA_S12M_TIMECODE,
+
+/**
+ * HDR dynamic metadata associated with a video frame. The payload is
+ * an AVDynamicHDRPlus type and contains information for color
+ * volume transform - application 4 of SMPTE 2094-40:2016 standard.
+ */
+AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
};
enum AVActiveFormatDescription {
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
new file mode 100644
index 00..7e4d7dec10
--- /dev/null
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -0,0 +1,47 @@
+/**
+ * Copyright (c) 2018 Mohammad Izadi
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "hdr_dynamic_metadata.h"
+#include "mem.h"
+
+AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
+{
+AVDynamicHDRPlus *hdr_plus = av_mallocz(sizeof(AVDynamicHDRPlus));
+if (!hdr_plus)
+return NULL;
+
+if (size)
+*size = sizeof(*hdr_plus);
+
+return hdr_plus;
+}
+
+AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame)
+{
+AVFrameSideData *side_data = av_frame_new_side_data(frame,
+
AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
+
sizeof(AVDynamicHDRPlus));
+if (!side_data)
+return NULL;
+
+memset(side_data->data, 0, sizeof(AVDynamicHDRPlus));
+
+return (AVDynamicHDRPlus *)side_data->data;
+}
diff --git
Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.
Thank you! Please go ahead and push the patch. -- Best, Mohammad On Thu, Dec 20, 2018 at 11:07 AM Vittorio Giovara < vittorio.giov...@gmail.com> wrote: > On Thu, Dec 20, 2018 at 7:18 PM Mohammad Izadi > wrote: > > > Hi Vittorio, > > > > Thank you for your feedback ! Here is my answers to your questions: > > > > I thought we were going to rename the header as dynamic_hdr.h since it > may > > contain multiple variants of metadata. > > Also I believe "metadata" in the name is redundant, but won't insist too > > much if you have strong feelings for it. > > *dynamic_hdr does not really point to dynamic metadata. When we use > > dynamic_hdr, it may be interpreted as a new type of HDR. I think > > hdr_dynamic_metadata is more meaningful. * > > > > ok i won't insist about this too much if you prefer > > > > Since these two types only apply to HDR, do you think adding HDR in their > > names, like AVHDRPlusOverlapProcessOption and AVHDRPlusPercentile, would > > make sense? > > Would make them similar to the other types below > > *Done.* > > > > maybe add "or NULL on failure." here too > > also why return type and function name on two different lines? > > *Done.* > > > > cool thanks > by the way do you have commit access or would like me to push the updated > patch when you send it? > -- > Vittorio > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.
On Thu, Dec 20, 2018 at 7:18 PM Mohammad Izadi wrote: > Hi Vittorio, > > Thank you for your feedback ! Here is my answers to your questions: > > I thought we were going to rename the header as dynamic_hdr.h since it may > contain multiple variants of metadata. > Also I believe "metadata" in the name is redundant, but won't insist too > much if you have strong feelings for it. > *dynamic_hdr does not really point to dynamic metadata. When we use > dynamic_hdr, it may be interpreted as a new type of HDR. I think > hdr_dynamic_metadata is more meaningful. * > ok i won't insist about this too much if you prefer > Since these two types only apply to HDR, do you think adding HDR in their > names, like AVHDRPlusOverlapProcessOption and AVHDRPlusPercentile, would > make sense? > Would make them similar to the other types below > *Done.* > > maybe add "or NULL on failure." here too > also why return type and function name on two different lines? > *Done.* > cool thanks by the way do you have commit access or would like me to push the updated patch when you send it? -- Vittorio ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.
Hi Vittorio,
Thank you for your feedback ! Here is my answers to your questions:
I thought we were going to rename the header as dynamic_hdr.h since it may
contain multiple variants of metadata.
Also I believe "metadata" in the name is redundant, but won't insist too
much if you have strong feelings for it.
*dynamic_hdr does not really point to dynamic metadata. When we use
dynamic_hdr, it may be interpreted as a new type of HDR. I think
hdr_dynamic_metadata is more meaningful. *
Since these two types only apply to HDR, do you think adding HDR in their
names, like AVHDRPlusOverlapProcessOption and AVHDRPlusPercentile, would
make sense?
Would make them similar to the other types below
*Done.*
maybe add "or NULL on failure." here too
also why return type and function name on two different lines?
*Done.*
--
Best,
Mohammad
On Wed, Dec 19, 2018 at 11:35 AM Vittorio Giovara <
vittorio.giov...@gmail.com> wrote:
> On Wed, Dec 19, 2018 at 7:08 PM Mohammad Izadi
> wrote:
>
> > From: Mohammad Izadi
> >
> > The dynamic metadata contains data for color volume transform -
> > application 4 of SMPTE 2094-40:2016 standard. The data comes from HEVC in
> > the SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35.
> >
> > I'll add support to HEVC in a follow-up.
> > ---
> >
>
> Thanks for the updated patch, here are some more comments below.
>
>
> > libavutil/Makefile | 2 +
> > libavutil/frame.c| 1 +
> > libavutil/frame.h| 7 +
> > libavutil/hdr_dynamic_metadata.c | 47 +
> > libavutil/hdr_dynamic_metadata.h | 344 +++
> > libavutil/version.h | 4 +-
> > 6 files changed, 403 insertions(+), 2 deletions(-)
> > create mode 100644 libavutil/hdr_dynamic_metadata.c
> > create mode 100644 libavutil/hdr_dynamic_metadata.h
> >
>
> I thought we were going to rename the header as dynamic_hdr.h since it may
> contain multiple variants of metadata.
> Also I believe "metadata" in the name is redundant, but won't insist too
> much if you have strong feelings for it.
>
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> > index 66f27f44bd..582ac470b2 100644
> > --- a/libavutil/frame.h
> > +++ b/libavutil/frame.h
> > @@ -166,6 +166,13 @@ enum AVFrameSideDataType {
> > * function in libavutil/timecode.c.
> > */
> > AV_FRAME_DATA_S12M_TIMECODE,
> > +
> > +/**
> > + * HDR dynamic metadata associated with a video frame. The payload
> is
> > + * an AVDynamicHDRPlus type and contains information for color
> > + * volume transform - application 4 of SMPTE 2094-40:2016 standard.
> > + */
> > +AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
> > };
> >
> > enum AVActiveFormatDescription {
> > diff --git a/libavutil/hdr_dynamic_metadata.c
> > b/libavutil/hdr_dynamic_metadata.c
> > new file mode 100644
> > index 00..7e4d7dec10
> > --- /dev/null
> > +++ b/libavutil/hdr_dynamic_metadata.c
> > @@ -0,0 +1,47 @@
> > +/**
> > + * Copyright (c) 2018 Mohammad Izadi
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > 02110-1301 USA
> > + */
> > +
> > +#include "hdr_dynamic_metadata.h"
> > +#include "mem.h"
> > +
> > +AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
> > +{
> > +AVDynamicHDRPlus *hdr_plus = av_mallocz(sizeof(AVDynamicHDRPlus));
> > +if (!hdr_plus)
> > +return NULL;
> > +
> > +if (size)
> > +*size = sizeof(*hdr_plus);
> > +
> > +return hdr_plus;
> > +}
> >
>
> this looks good
>
> + * Option for overlapping elliptical pixel selectors in an image.
> > + */
> > +enum AVOverlapProcessOption {
> > +AV_OVERLAP_PROCESS_WEIGHTED_AVERAGING = 0,
> > +AV_OVERLAP_PROCESS_LAYERING = 1,
> > +};
> > +
> > +/**
> > + * Percentile represents the percentile at a specific percentage in
> > + * a distribution.
> > + */
> > +typedef struct AVPercentile {
> > +/**
> > + * The percentage value corresponding to a specific percentile
> > linearized
> > + * RGB value in the processing window in the scene. The value shall
> > be in
> > + * the range of 0 to100, inclusive.
> > + */
> > +uint8_t percentage;
> > +/**
> > + * The linearized maxRGB value at a specific percentile in the
> > processing
Re: [FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.
On Wed, Dec 19, 2018 at 7:08 PM Mohammad Izadi wrote:
> From: Mohammad Izadi
>
> The dynamic metadata contains data for color volume transform -
> application 4 of SMPTE 2094-40:2016 standard. The data comes from HEVC in
> the SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35.
>
> I'll add support to HEVC in a follow-up.
> ---
>
Thanks for the updated patch, here are some more comments below.
> libavutil/Makefile | 2 +
> libavutil/frame.c| 1 +
> libavutil/frame.h| 7 +
> libavutil/hdr_dynamic_metadata.c | 47 +
> libavutil/hdr_dynamic_metadata.h | 344 +++
> libavutil/version.h | 4 +-
> 6 files changed, 403 insertions(+), 2 deletions(-)
> create mode 100644 libavutil/hdr_dynamic_metadata.c
> create mode 100644 libavutil/hdr_dynamic_metadata.h
>
I thought we were going to rename the header as dynamic_hdr.h since it may
contain multiple variants of metadata.
Also I believe "metadata" in the name is redundant, but won't insist too
much if you have strong feelings for it.
diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 66f27f44bd..582ac470b2 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -166,6 +166,13 @@ enum AVFrameSideDataType {
> * function in libavutil/timecode.c.
> */
> AV_FRAME_DATA_S12M_TIMECODE,
> +
> +/**
> + * HDR dynamic metadata associated with a video frame. The payload is
> + * an AVDynamicHDRPlus type and contains information for color
> + * volume transform - application 4 of SMPTE 2094-40:2016 standard.
> + */
> +AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
> };
>
> enum AVActiveFormatDescription {
> diff --git a/libavutil/hdr_dynamic_metadata.c
> b/libavutil/hdr_dynamic_metadata.c
> new file mode 100644
> index 00..7e4d7dec10
> --- /dev/null
> +++ b/libavutil/hdr_dynamic_metadata.c
> @@ -0,0 +1,47 @@
> +/**
> + * Copyright (c) 2018 Mohammad Izadi
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> 02110-1301 USA
> + */
> +
> +#include "hdr_dynamic_metadata.h"
> +#include "mem.h"
> +
> +AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
> +{
> +AVDynamicHDRPlus *hdr_plus = av_mallocz(sizeof(AVDynamicHDRPlus));
> +if (!hdr_plus)
> +return NULL;
> +
> +if (size)
> +*size = sizeof(*hdr_plus);
> +
> +return hdr_plus;
> +}
>
this looks good
+ * Option for overlapping elliptical pixel selectors in an image.
> + */
> +enum AVOverlapProcessOption {
> +AV_OVERLAP_PROCESS_WEIGHTED_AVERAGING = 0,
> +AV_OVERLAP_PROCESS_LAYERING = 1,
> +};
> +
> +/**
> + * Percentile represents the percentile at a specific percentage in
> + * a distribution.
> + */
> +typedef struct AVPercentile {
> +/**
> + * The percentage value corresponding to a specific percentile
> linearized
> + * RGB value in the processing window in the scene. The value shall
> be in
> + * the range of 0 to100, inclusive.
> + */
> +uint8_t percentage;
> +/**
> + * The linearized maxRGB value at a specific percentile in the
> processing
> + * window in the scene. The value shall be in the range of 0 to 1,
> inclusive
> + * and in multiples of 0.1.
> + */
> +AVRational percentile;
> +} AVPercentile;
>
Since these two types only apply to HDR, do you think adding HDR in their
names, like AVHDRPlusOverlapProcessOption and AVHDRPlusPercentile, would
make sense?
Would make them similar to the other types below
+
> +/**
> + * Color transform parameters at a processing window in a dynamic
> metadata for
> + * SMPTE 2094-40.
> + */
> +typedef struct AVHDRPlusColorTransformParams {
> +/**
> + * The relative x coordinate of the top left pixel of the processing
> + * window. The value shall be in the range of 0 and 1, inclusive and
> + * in multiples of 1/(width of Picture – 1). The value 1 corresponds
> + * to the absolute coordinate of width of Picture – 1. The value for
> + * first processing window shall be 0.
> + */
> +AVRational window_upper_left_corner_x;
> +
> +/**
> + * The relative y coordinate of the top left pixel of the processing
> + * window. The value shall be in the range of 0 and 1, inclusive and
> +
[FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.
From: Mohammad Izadi
The dynamic metadata contains data for color volume transform - application 4
of SMPTE 2094-40:2016 standard. The data comes from HEVC in the
SEI_TYPE_USER_DATA_REGISTERED_ITU_T_T35.
I'll add support to HEVC in a follow-up.
---
libavutil/Makefile | 2 +
libavutil/frame.c| 1 +
libavutil/frame.h| 7 +
libavutil/hdr_dynamic_metadata.c | 47 +
libavutil/hdr_dynamic_metadata.h | 344 +++
libavutil/version.h | 4 +-
6 files changed, 403 insertions(+), 2 deletions(-)
create mode 100644 libavutil/hdr_dynamic_metadata.c
create mode 100644 libavutil/hdr_dynamic_metadata.h
diff --git a/libavutil/Makefile b/libavutil/Makefile
index caddc7e155..7dcb92b06b 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -31,6 +31,7 @@ HEADERS = adler32.h
\
file.h\
frame.h \
hash.h\
+ hdr_dynamic_metadata.h\
hmac.h\
hwcontext.h \
hwcontext_cuda.h \
@@ -119,6 +120,7 @@ OBJS = adler32.o
\
fixed_dsp.o \
frame.o \
hash.o \
+ hdr_dynamic_metadata.o \
hmac.o \
hwcontext.o \
imgutils.o \
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 9b3fb13e68..34a6210d9e 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -840,6 +840,7 @@ const char *av_frame_side_data_name(enum
AVFrameSideDataType type)
case AV_FRAME_DATA_QP_TABLE_PROPERTIES: return "QP table
properties";
case AV_FRAME_DATA_QP_TABLE_DATA: return "QP table data";
#endif
+case AV_FRAME_DATA_DYNAMIC_HDR_PLUS: return "HDR Dynamic Metadata
SMPTE2094-40 (HDR10+)";
}
return NULL;
}
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 66f27f44bd..582ac470b2 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -166,6 +166,13 @@ enum AVFrameSideDataType {
* function in libavutil/timecode.c.
*/
AV_FRAME_DATA_S12M_TIMECODE,
+
+/**
+ * HDR dynamic metadata associated with a video frame. The payload is
+ * an AVDynamicHDRPlus type and contains information for color
+ * volume transform - application 4 of SMPTE 2094-40:2016 standard.
+ */
+AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
};
enum AVActiveFormatDescription {
diff --git a/libavutil/hdr_dynamic_metadata.c b/libavutil/hdr_dynamic_metadata.c
new file mode 100644
index 00..7e4d7dec10
--- /dev/null
+++ b/libavutil/hdr_dynamic_metadata.c
@@ -0,0 +1,47 @@
+/**
+ * Copyright (c) 2018 Mohammad Izadi
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "hdr_dynamic_metadata.h"
+#include "mem.h"
+
+AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size)
+{
+AVDynamicHDRPlus *hdr_plus = av_mallocz(sizeof(AVDynamicHDRPlus));
+if (!hdr_plus)
+return NULL;
+
+if (size)
+*size = sizeof(*hdr_plus);
+
+return hdr_plus;
+}
+
+AVDynamicHDRPlus *av_dynamic_hdr_plus_create_side_data(AVFrame *frame)
+{
+AVFrameSideData *side_data = av_frame_new_side_data(frame,
+
AV_FRAME_DATA_DYNAMIC_HDR_PLUS,
+
sizeof(AVDynamicHDRPlus));
+if (!side_data)
+return NULL;
+
+memset(side_data->data, 0, sizeof(AVDynamicHDRPlus));
+
+return (AVDynamicHDRPlus *)side_data->data;
+}
diff --git
