On 9/20/2017 12:47 PM, Rostislav Pehlivanov wrote: > On 20 September 2017 at 16:05, Hendrik Leppkes <h.lepp...@gmail.com> 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 >> _______________________________________________ >> ffmpeg-devel mailing list >> ffmpeg-devel@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?
Because the standard could always get new values, i presume. In any case, as i said, the AVMasteringDisplayMetadata is currently kinda fucked because of the lack of a proper allocation function. A new one needs to be added in any case, and no new fields whatsoever should be added to the struct until a major bump takes place. > The standard sucks and I see no reason why we need to stick to it. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel