[FFmpeg-devel] [PATCH] libavutil: add mastering display metadata sidedata

Neil Birkbeck neil.birkbeck at gmail.com
Thu Jan 21 06:40:29 CET 2016


Thanks for the detailed responses and discussions, and apologies for
delayed response (I was considering to change the functions to a
serialize/deserialize to some wire format and keeping the struct, but wire
format would've essentially be a memcpy to the struct on little endian and
would only benefit if there was a place that was generically writing the
SideData's data to file--wasn't sure if that was happening anywhere).

On Tue, Jan 19, 2016 at 7:39 AM, Michael Niedermayer <michael at niedermayer.cc
> wrote:

> On Tue, Jan 19, 2016 at 01:34:12PM +0100, wm4 wrote:
> > On Tue, 19 Jan 2016 13:02:25 +0100
> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> >
> > > On Tue, Jan 19, 2016 at 10:18:16AM +0100, wm4 wrote:
> > > > On Tue, 19 Jan 2016 00:32:43 +0100
> > > > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > > >
> > > > > On Sat, Jan 16, 2016 at 04:19:38PM -0800, Neil Birkbeck wrote:
> > > > > > Adding mastering display metadata struct to avutil. The
> mastering display metadata contains information
> > > > > > about the mastering display color volume (SMPTE 2086:2014).
> > > > > >
> > > > > > This info comes from HEVC in the SEI_TYPE_MASTERING_DISPLAY_INFO
> and is soon to be included in MKV:
> > > > > >
> https://mailarchive.ietf.org/arch/search/?email_list=cellar&gbt=1&index=sZyfPTM-QY69P-0omfOIiTN622o
> > > > > > so it is similar to SEI FPA / stereo_mode in MKV and as such
> this patch follows how AVStereo3D is implemented.
> > > > > >
> > > > > > I'll add support to HEVC in a follow-up (and MKV when spec is
> approved).
> > > > > >
> > > > > > Signed-off-by: Neil Birkbeck <neil.birkbeck at gmail.com>
> > > > > > ---
> > > > > >  libavutil/Makefile                     |  2 +
> > > > > >  libavutil/frame.h                      |  7 ++-
> > > > > >  libavutil/mastering_display_metadata.c | 43 +++++++++++++++++
> > > > > >  libavutil/mastering_display_metadata.h | 87
> ++++++++++++++++++++++++++++++++++
> > > > > >  libavutil/version.h                    |  2 +-
> > > > > >  5 files changed, 139 insertions(+), 2 deletions(-)
> > > > > >  create mode 100644 libavutil/mastering_display_metadata.c
> > > > > >  create mode 100644 libavutil/mastering_display_metadata.h
> > > > > >
> > > > > > diff --git a/libavutil/Makefile b/libavutil/Makefile
> > > > > > index bf8c713..65b2d25 100644
> > > > > > --- a/libavutil/Makefile
> > > > > > +++ b/libavutil/Makefile
> > > > > > @@ -38,6 +38,7 @@ HEADERS = adler32.h
>                          \
> > > > > >            log.h
>          \
> > > > > >            macros.h
>         \
> > > > > >            mathematics.h
>          \
> > > > > > +          mastering_display_metadata.h
>         \
> > > > > >            md5.h
>          \
> > > > > >            mem.h
>          \
> > > > > >            motion_vector.h
>          \
> > > > > > @@ -115,6 +116,7 @@ OBJS = adler32.o
>                             \
> > > > > >         log.o
>         \
> > > > > >         log2_tab.o
>          \
> > > > > >         mathematics.o
>         \
> > > > > > +       mastering_display_metadata.o
>          \
> > > > > >         md5.o
>         \
> > > > > >         mem.o
>         \
> > > > > >         murmur3.o
>         \
> > > > > > diff --git a/libavutil/frame.h b/libavutil/frame.h
> > > > > > index 9c6061a..308355b 100644
> > > > > > --- a/libavutil/frame.h
> > > > > > +++ b/libavutil/frame.h
> > > > > > @@ -106,12 +106,17 @@ enum AVFrameSideDataType {
> > > > > >       * @endcode
> > > > > >       */
> > > > > >      AV_FRAME_DATA_SKIP_SAMPLES,
> > > > > > -
> > > > > >      /**
> > > > > >       * This side data must be associated with an audio frame
> and corresponds to
> > > > > >       * enum AVAudioServiceType defined in avcodec.h.
> > > > > >       */
> > > > > >      AV_FRAME_DATA_AUDIO_SERVICE_TYPE,
> > > > >
> > > > > > +    /**
> > > > > > +     * Mastering display metadata associated with a video
> frame. The payload is
> > > > > > +     * an AVMasteringDisplayMetadata type and contains
> information about the
> > > > > > +     * mastering display color volume.
> > > > > > +     */
> > > > > > +    AV_FRAME_DATA_MASTERING_DISPLAY_METADATA
> > > > >
> > > > > will this always stay the same or could it require to be extended
> in
> > > > > the future ?
> > > > > if future extensions may occur then it should be documented how
> that
> > > > > would work. Would fields be added to the end of the struct ?
> > > > > If so how would this be detected ?
> > > > >
> > > > > C does not gurantee the sizeof to change for the addition of fields
> > > > > for example these 2 are both 8bytes on x86-64
> > > > > typedef struct X { int a; char b; }X;
> > > > > typedef struct Y { int a; char b; char c; }Y;
> > > > >
> > > > > defining the meaning of the sidedata byte per byte avoids the
> problem
> > > > > of detectability it also makes the function endian and ABI
> independant
> > > > > and would allow passing it bytewise between computers, no idea how
> > > > > much thats a real world use case though
> > > > >
> > > > > [...]
> > > >
> > > > I thought we decided side data can be memcpy'd struct? (I.e. can be
> > > > ABI-dependent.)
> > >
> > > i dont remember
> >
> > Well, we have e.g. AVStereo3D in AV_FRAME_DATA_STEREO3D.
> >
> > > but more importantly, how would such struct be extended, if we
> > > want to add a field ?
> >
> > Like we always do, append a field at the end.
> >
> > > searching for AV_FRAME_DATA* finds hits in libavcodec, libavdevice,
> > > libavfilter, ffprobe as well as doc/examples
> > >
> > > adding a field is not guranteed to change the sizeof() of the struct
> > > so using the sidedatas size to detect the version would be undefined
> > > behavior also the exact size could with structs differ between
> > > platforms making compatibility code potentially a mess.
> >
> > True, but this is also true for all other structs we use everywhere.
>
> true, patch applied
>
> thx
>
> [...]
> --
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Its not that you shouldnt use gotos but rather that you should write
> readable code and code with gotos often but not always is less readable
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>


More information about the ffmpeg-devel mailing list