[FFmpeg-devel] [PATCH 1/2] Support HDR dynamic metdata (HDR10+) in HEVC decoder.

[James Almer]

On 1/4/2019 2:51 PM, Mohammad Izadi wrote:
> I did that before and James ask me to copy. So now, James are you OK with
> that?
Your first patch allocated a second AVDynamicHDRPlus and memcpy'd
everything to the new one every frame. Rostislav suggestion is to
allocate a single AVDynamicHDRPlus, make an AVBufferRef out of it, then
attach a new reference to it as side data on every frame.

And yes, I'm ok with that if the SEI is valid for every frame after it.
It will be much faster and cleaner.

> --
> Best,
> Mohammad
> 
> 
> On Thu, Jan 3, 2019 at 5:08 PM Rostislav Pehlivanov <atomnuker at gmail.com>
> wrote:
> 
>> On Thu, 3 Jan 2019 at 20:13, Mohammad Izadi <moh.izadi at gmail.com> wrote:
>>
>>>
>>>  /**
>>> @@ -94,6 +95,50 @@ typedef struct HEVCSEIMasteringDisplay {
>>>      uint32_t min_luminance;
>>>  } HEVCSEIMasteringDisplay;
>>>
>>> +typedef struct HEVCSEIDynamicHDRPlus{
>>> +    int present;
>>> +    uint8_t itu_t_t35_country_code;
>>> +    uint8_t application_version;
>>> +    uint8_t num_windows;
>>> +    struct {
>>> +        AVRational window_upper_left_corner_x;
>>> +        AVRational window_upper_left_corner_y;
>>> +        AVRational window_lower_right_corner_x;
>>> +        AVRational window_lower_right_corner_y;
>>> +        uint16_t center_of_ellipse_x;
>>> +        uint16_t center_of_ellipse_y;
>>> +        uint8_t rotation_angle;
>>> +        uint16_t semimajor_axis_internal_ellipse;
>>> +        uint16_t semimajor_axis_external_ellipse;
>>> +        uint16_t semiminor_axis_external_ellipse;
>>> +        uint8_t overlap_process_option;
>>> +        AVRational maxscl[3];
>>> +        AVRational average_maxrgb;
>>> +        uint8_t num_distribution_maxrgb_percentiles;
>>> +        struct {
>>> +            uint8_t percentage;
>>> +            AVRational percentile;
>>> +        } distribution_maxrgb[15];
>>> +        AVRational fraction_bright_pixels;
>>> +        uint8_t tone_mapping_flag;
>>> +        AVRational knee_point_x;
>>> +        AVRational knee_point_y;
>>> +        uint8_t num_bezier_curve_anchors;
>>> +        AVRational bezier_curve_anchors[15];
>>> +        uint8_t color_saturation_mapping_flag;
>>> +        AVRational color_saturation_weight;
>>> +    } params[3];
>>> +    AVRational targeted_system_display_maximum_luminance;
>>> +    uint8_t targeted_system_display_actual_peak_luminance_flag;
>>> +    uint8_t num_rows_targeted_system_display_actual_peak_luminance;
>>> +    uint8_t num_cols_targeted_system_display_actual_peak_luminance;
>>> +    AVRational targeted_system_display_actual_peak_luminance[25][25];
>>> +    uint8_t mastering_display_actual_peak_luminance_flag;
>>> +    uint8_t num_rows_mastering_display_actual_peak_luminance;
>>> +    uint8_t num_cols_mastering_display_actual_peak_luminance;
>>> +    AVRational mastering_display_actual_peak_luminance[25][25];
>>> +} HEVCSEIDynamicHDRPlus;
>>> +
>>>
>>
>> There's no reason to create a new struct for this if all you're going to do
>> is to copy it over and over to new frames.
>> What you can do is this: on every SEI containing this thing just allocate a
>> AVDynamicHDRPlus struct via av_dynamic_hdr_plus_alloc, wrap it as an
>> AVBufferRef via av_buffer_create, populate it, put it as a pointer in
>> HEVCSEIContentLight and then on every frame just add it to the frame via
>> av_frame_new_side_data_from_buf. If a new SEI is received unref your
>> pointer in HEVCSEIDynamicHDRPlus and repeat by allocating a new struct,
>> wrapping it as a buffer, populating it, etc.
>> I figure the only reason this wasn't done with other metadata was because
>> they were much smaller and because av_frame_new_side_data_from_buf didn't
>> exist until recently.
>>
>>
>> +            av_log(s->avctx, AV_LOG_DEBUG, ")");
>>> +            if (metadata->params[w].color_saturation_mapping_flag) {
>>> +                av_log(s->avctx, AV_LOG_DEBUG,
>>> +                       " color_saturation_weight=%5.4f",
>>> +
>>>  av_q2d(metadata->params[w].color_saturation_weight));
>>> +            }
>>> +            av_log(s->avctx, AV_LOG_DEBUG, "}\n");
>>> +        }
>>> +        av_log(s->avctx, AV_LOG_DEBUG,
>>> +               "} End of HDR10+ (SMPTE 2094-40)\n");
>>> +    }
>>>
>>
>> Don't spam stuff like than in the debug log, especially not on every single
>> frame. Tools exist to print side data so just don't.
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>