[FFmpeg-devel] [PATCH] Add HDR dynamic metadata struct (for SMPTE 2094-40) to libavutil.

Mohammad Izadi moh.izadi at gmail.com
Thu Dec 20 20:11:17 EET 2018


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.giovara at gmail.com> wrote:

> On Wed, Dec 19, 2018 at 7:08 PM Mohammad Izadi <moh.izadi at gmail.com>
> wrote:
>
> > From: Mohammad Izadi <izadi at google.com>
> >
> > 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 0000000000..7e4d7dec10
> > --- /dev/null
> > +++ b/libavutil/hdr_dynamic_metadata.c
> > @@ -0,0 +1,47 @@
> > +/**
> > + * Copyright (c) 2018 Mohammad Izadi <moh.izadi at gmail.com>
> > + *
> > + * 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.00001.
> > +     */
> > +    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
> > +     * in multiples of 1/(height of Picture – 1). The value 1
> corresponds
> > +     * to the absolute coordinate of height of Picture – 1. The value
> for
> > +     * first processing window shall be 0.
> > +     */
> > +    AVRational window_upper_left_corner_y;
> > +
> > +    /**
> > +     * The relative x coordinate of the bottom right 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 1.
> > +     */
> > +    AVRational window_lower_right_corner_x;
> > +
> > +    /**
> > +     * The relative y coordinate of the bottom right pixel of the
> > processing
> > +     * window. The value shall be in the range of 0 and 1, inclusive and
> > +     * in multiples of 1/(height of Picture – 1). The value 1
> corresponds
> > +     * to the absolute coordinate of height of Picture – 1. The value
> for
> > +     * first processing window shall be 1.
> > +     */
> > +    AVRational window_lower_right_corner_y;
> > +
> > +    /**
> > +     * The x coordinate of the center position of the concentric
> internal
> > and
> > +     * external ellipses of the elliptical pixel selector in the
> > processing
> > +     * window. The value shall be in the range of 0 to (width of Picture
> > – 1),
> > +     * inclusive and in multiples of 1 pixel.
> > +     */
> > +    uint16_t center_of_ellipse_x;
> > +
> > +    /**
> > +     * The y coordinate of the center position of the concentric
> internal
> > and
> > +     * external ellipses of the elliptical pixel selector in the
> > processing
> > +     * window. The value shall be in the range of 0 to (height of
> Picture
> > – 1),
> > +     * inclusive and in multiples of 1 pixel.
> > +     */
> > +    uint16_t center_of_ellipse_y;
> > +
> > +    /**
> > +     * The clockwise rotation angle in degree of arc with respect to the
> > +     * positive direction of the x-axis of the concentric internal and
> > external
> > +     * ellipses of the elliptical pixel selector in the processing
> > window. The
> > +     * value shall be in the range of 0 to 180, inclusive and in
> > multiples of 1.
> > +     */
> > +    uint8_t rotation_angle;
> > +
> > +    /**
> > +     * The semi-major axis value of the internal ellipse of the
> > elliptical pixel
> > +     * selector in amount of pixels in the processing window. The value
> > shall be
> > +     * in the range of 1 to 65535, inclusive and in multiples of 1
> pixel.
> > +     */
> > +    uint16_t semimajor_axis_internal_ellipse;
> > +
> > +    /**
> > +     * The semi-major axis value of the external ellipse of the
> > elliptical pixel
> > +     * selector in amount of pixels in the processing window. The value
> > +     * shall not be less than semimajor_axis_internal_ellipse of the
> > current
> > +     * processing window. The value shall be in the range of 1 to 65535,
> > +     * inclusive and in multiples of 1 pixel.
> > +     */
> > +    uint16_t semimajor_axis_external_ellipse;
> > +
> > +    /**
> > +     * The semi-minor axis value of the external ellipse of the
> > elliptical pixel
> > +     * selector in amount of pixels in the processing window. The value
> > shall be
> > +     * in the range of 1 to 65535, inclusive and in multiples of 1
> pixel.
> > +     */
> > +    uint16_t semiminor_axis_external_ellipse;
> > +
> > +    /**
> > +     * Overlap process option indicates one of the two methods of
> > combining
> > +     * rendered pixels in the processing window in an image with at
> least
> > one
> > +     * elliptical pixel selector. For overlapping elliptical pixel
> > selectors
> > +     * in an image, overlap_process_option shall have the same value.
> > +     */
> > +    enum AVOverlapProcessOption overlap_process_option;
> > +
> > +    /**
> > +     * The maximum of the color components of linearized RGB values in
> the
> > +     * processing window in the scene. The values should be in the range
> > of 0 to
> > +     * 1, inclusive and in multiples of 0.00001. maxscl[ 0 ], maxscl[ 1
> > ], and
> > +     * maxscl[ 2 ] are corresponding to R, G, B color components
> > respectively.
> > +     */
> > +    AVRational maxscl[3];
> > +
> > +    /**
> > +     * The average of linearized maxRGB values in the processing window
> > in the
> > +     * scene. The value should be in the range of 0 to 1, inclusive and
> in
> > +     * multiples of 0.00001.
> > +     */
> > +    AVRational average_maxrgb;
> > +
> > +    /**
> > +     * The number of linearized maxRGB values at given percentiles in
> the
> > +     * processing window in the scene. The maximum value shall be 15.
> > +     */
> > +    uint8_t num_distribution_maxrgb_percentiles;
> > +
> > +    /**
> > +     * The linearized maxRGB values at given percentiles in the
> > +     * processing window in the scene.
> > +     */
> > +    AVPercentile distribution_maxrgb[15];
> > +
> > +    /**
> > +     * The fraction of selected pixels in the image that contains the
> > brightest
> > +     * pixel in the scene. The value shall be in the range of 0 to 1,
> > inclusive
> > +     * and in multiples of 0.001.
> > +     */
> > +    AVRational fraction_bright_pixels;
> > +
> > +    /**
> > +     * This flag indicates that the metadata for the tone mapping
> > function in
> > +     * the processing window is present (for value of 1).
> > +     */
> > +    uint8_t tone_mapping_flag;
> > +
> > +    /**
> > +     * The x coordinate of the separation point between the linear part
> > and the
> > +     * curved part of the tone mapping function. The value shall be in
> > the range
> > +     * of 0 to 1, excluding 0 and in multiples of 1/4095.
> > +     */
> > +    AVRational knee_point_x;
> > +
> > +    /**
> > +     * The y coordinate of the separation point between the linear part
> > and the
> > +     * curved part of the tone mapping function. The value shall be in
> > the range
> > +     * of 0 to 1, excluding 0 and in multiples of 1/4095.
> > +     */
> > +    AVRational knee_point_y;
> > +
> > +    /**
> > +     * The number of the intermediate anchor parameters of the tone
> > mapping
> > +     * function in the processing window. The maximum value shall be 15.
> > +     */
> > +    uint8_t num_bezier_curve_anchors;
> > +
> > +    /**
> > +     * The intermediate anchor parameters of the tone mapping function
> in
> > the
> > +     * processing window in the scene. The values should be in the range
> > of 0
> > +     * to 1, inclusive and in multiples of 1/1023.
> > +     */
> > +    AVRational bezier_curve_anchors[15];
> > +
> > +    /**
> > +     * This flag shall be equal to 0 in bitstreams conforming to this
> > version of
> > +     * this Specification. Other values are reserved for future use.
> > +     */
> > +    uint8_t color_saturation_mapping_flag;
> > +
> > +    /**
> > +     * The color saturation gain in the processing window in the scene.
> > The
> > +     * value shall be in the range of 0 to 63/8, inclusive and in
> > multiples of
> > +     * 1/8. The default value shall be 1.
> > +     */
> > +    AVRational color_saturation_weight;
> > +} AVHDRPlusColorTransformParams;
> > +
> > +/**
> > + * This struct represents dynamic metadata for color volume transform -
> > + * application 4 of SMPTE 2094-40:2016 standard.
> > + *
> > + * To be used as payload of a AVFrameSideData or AVPacketSideData with
> the
> > + * appropriate type.
> > + *
> > + * @note The struct should be allocated with
> > + * av_dynamic_hdr_plus_alloc() and its size is not a part of
> > + * the public ABI.
> > + */
> > +typedef struct AVDynamicHDRPlus {
> > +    /**
> > +     * Country code by Rec. ITU-T T.35 Annex A. The value shall be 0xB5.
> > +     */
> > +    uint8_t itu_t_t35_country_code;
> > +
> > +    /**
> > +     * Application version in the application defining document in
> ST-2094
> > +     * suite. The value shall be set to 0.
> > +     */
> > +    uint8_t application_version;
> > +
> > +    /**
> > +     * The number of processing windows. The value shall be in the range
> > +     * of 1 to 3, inclusive.
> > +     */
> > +    uint8_t num_windows;
> > +
> > +    /**
> > +     * The color transform parameters for every processing window.
> > +     */
> > +    AVHDRPlusColorTransformParams params[3];
> > +
> > +    /**
> > +     * The nominal maximum display luminance of the targeted system
> > display,
> > +     * in units of 0.0001 candelas per square metre. The value shall be
> in
> > +     * the range of 0 to 10000, inclusive.
> > +     */
> > +    AVRational targeted_system_display_maximum_luminance;
> > +
> > +    /**
> > +     * This flag shall be equal to 0 in bit streams conforming to this
> > version
> > +     * of this Specification. The value 1 is reserved for future use.
> > +     */
> > +    uint8_t targeted_system_display_actual_peak_luminance_flag;
> > +
> > +    /**
> > +     * The number of rows in the targeted
> > system_display_actual_peak_luminance
> > +     * array. The value shall be in the range of 2 to 25, inclusive.
> > +     */
> > +    uint8_t num_rows_targeted_system_display_actual_peak_luminance;
> > +
> > +    /**
> > +     * The number of columns in the
> > +     * targeted_system_display_actual_peak_luminance array. The value
> > shall be
> > +     * in the range of 2 to 25, inclusive.
> > +     */
> > +    uint8_t num_cols_targeted_system_display_actual_peak_luminance;
> > +
> > +    /**
> > +     * The normalized actual peak luminance of the targeted system
> > display. The
> > +     * values should be in the range of 0 to 1, inclusive and in
> > multiples of
> > +     * 1/15.
> > +     */
> > +    AVRational targeted_system_display_actual_peak_luminance[25][25];
> > +
> > +    /**
> > +     * This flag shall be equal to 0 in bitstreams conforming to this
> > version of
> > +     * this Specification. The value 1 is reserved for future use.
> > +     */
> > +    uint8_t mastering_display_actual_peak_luminance_flag;
> > +
> > +    /**
> > +     * The number of rows in the mastering_display_actual_peak_luminance
> > array.
> > +     * The value shall be in the range of 2 to 25, inclusive.
> > +     */
> > +    uint8_t num_rows_mastering_display_actual_peak_luminance;
> > +
> > +    /**
> > +     * The number of columns in the
> > mastering_display_actual_peak_luminance
> > +     * array. The value shall be in the range of 2 to 25, inclusive.
> > +     */
> > +    uint8_t num_cols_mastering_display_actual_peak_luminance;
> > +
> > +    /**
> > +     * The normalized actual peak luminance of the mastering display
> used
> > for
> > +     * mastering the image essence. The values should be in the range of
> > 0 to 1,
> > +     * inclusive and in multiples of 1/15.
> > +     */
> > +    AVRational mastering_display_actual_peak_luminance[25][25];
> > +
> > +} AVDynamicHDRPlus;
> > +
> > +/**
> > + * Allocate an AVDynamicHDRPlus structure and set its fields to
> > + * default values. The resulting struct can be freed using av_freep().
> > + *
> > + * @return An AVDynamicHDRPlus filled with default values or NULL
> > + *         on failure.
> > + */
> > +AVDynamicHDRPlus *av_dynamic_hdr_plus_alloc(size_t *size);
> > +
> > +/**
> > + * Allocate a complete AVDynamicHDRPlus and add it to the frame.
> > + *
> > + * @param frame The frame which side data is added to.
> > + *
> > + * @return The AVDynamicHDRPlus structure to be filled by caller.
> > + */
> > +AVDynamicHDRPlus *
> > +av_dynamic_hdr_plus_create_side_data(AVFrame *frame);
> >
>
> maybe add "or NULL on failure." here too
> also why return type and function name on two different lines?
>
>
> > +
> > +#endif /* AVUTIL_HDR_DYNAMIC_METADATA_H */
> >
>
>
> --
> Vittorio
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


More information about the ffmpeg-devel mailing list