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

[Vittorio Giovara]

>* 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>
*>* http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
<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.


"mastering display" is a specification defined in several standards,
including h264 and h265. It groups two kind of properties, the mastering
display primaries (with its whitepoint) and the screen min and max
luminance. AVMasteringDisplayMetadata is modelled after this specification
and designed to expose the same kind of information; setters and getter
will expect to use all the information contained in it.

Adding a field that is missing from the original specification to that
structure is, in my opinion, semantically wrong, not only because
AVMasteringDisplayMetadata won't be strictly mapped to a the "mastering
display" any more, but also because the setters and getters of the
"mastering display" users will not need the new field at all.

There is a precedent to this: AVColorVolume. Its field are, in practice,
tied to mastering display, but they are stored in a separate structure, and
not squashed in the main AVMasteringDisplayMetadata structure. I think we
should continue this trend so that we can easily map structures with known
specifications and not overload them with unnecessary fields.
-- 
Vittorio