[FFmpeg-devel] [PATCH 1/2] lavu: add a gamma field to AVMasteringDisplayMetadata

[James Almer]

On 9/20/2017 12:05 PM, Hendrik Leppkes wrote:
> On Wed, Sep 20, 2017 at 4:34 PM, Rostislav Pehlivanov
> <atomnuker at gmail.com> wrote:
>> On 20 September 2017 at 12:55, Vittorio Giovara <vittorio.giovara at gmail.com>
>> wrote:
>>
>>> diff --git a/libavutil/mastering_display_metadata.h
>>> b/libavutil/mastering_display_metadata.h
>>> index 847b0b62c6..3de58bf468 100644
>>> --- a/libavutil/mastering_display_metadata.h
>>> +++ b/libavutil/mastering_display_metadata.h
>>> @@ -66,6 +66,16 @@ typedef struct AVMasteringDisplayMetadata {
>>>       */
>>>      int has_luminance;
>>>
>>> +    /**
>>> +     * The power-law response exponent needed to compensate for
>>> nonlinearity.
>>> +     */
>>> +    AVRational gamma;
>>> +
>>> +    /**
>>> +     * Flag indicating whether the gamma has been set.
>>> +     */
>>> +    int has_gamma;
>>> +
>>>  } AVMasteringDisplayMetadata;
>>>
>>>
>>> In my opinion we should not add new fields to structures that map 1:1 to
>>> something defined elsewhere (with the exception of booleans).
>>> I think this should be a separate side data and type, ie AVGammaResponse,
>>> that can of course reside in the same header.
>>> --
>>> Vittorio
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel at ffmpeg.org
>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>>
>>
>> Why? I don't see anything special about the struct. And this value fits in
>> exactly to what the struct is all about.
> 
> I basically agree with Vittorio, the struct basically represents the
> HDR metadata as specified for SMPTE ST 2086 (and as signaled as such
> in HEVC and various container formats). Jumbling other values in which
> are not part of that standard doesn't seem ideal.
> 
> - Hendrik

+1.

I very much rather not touch AVMasteringDisplayMetadata at all. The
struct size is supposedly not part of the ABI yet the alloc function
doesn't report the size in any way whatsoever, resulting in the need to
use sizeof() outside of lavu. Adding new values will inevitably lead to
backwards compatibility issues.

Besides, the frame/packet side data API is extremely easy to expand. You
just add a new type, and when needed a struct and accompanying utility
functions.