On 9/20/2017 12:05 PM, Hendrik Leppkes wrote: > On Wed, Sep 20, 2017 at 4:34 PM, Rostislav Pehlivanov > <atomnu...@gmail.com> wrote: >> On 20 September 2017 at 12:55, Vittorio Giovara <vittorio.giov...@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@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. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel