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

[Rostislav Pehlivanov]

On 20 September 2017 at 16:05, Hendrik Leppkes <h.leppkes at gmail.com> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

So why not name the whole struct with some hevc prefix and add a field
saying: "New values are forbidden because SMPTE said so!", rather than a
warning saying not to use the size of the struct as a public API because
new values might be added?
The standard sucks and I see no reason why we need to stick to it.