[FFmpeg-devel] [PATCH] Update HDR10+ metadata structure.

Mohammad Izadi moh.izadi at gmail.com
Fri Feb 21 20:07:51 EET 2020


If you believe lavc is at the top of the hierarchy, I can simply move the
file to lavc. Then both lavc and lavf can use it and hierarchy is
respected. Can I do that? Doesn't break API? Any objection (with solution)?
Let's make right decisions to speed up the process please :).
--
Best,
Mohammad


On Fri, Feb 21, 2020 at 2:53 AM Hendrik Leppkes <h.leppkes at gmail.com> wrote:

> On Fri, Feb 21, 2020 at 5:59 AM Vittorio Giovara
> <vittorio.giovara at gmail.com> wrote:
> >
> > On Thu, Feb 20, 2020 at 6:38 PM James Almer <jamrial at gmail.com> wrote:
> >
> > > On 2/20/2020 5:02 PM, Vittorio Giovara wrote:
> > > > On Mon, Feb 10, 2020 at 3:14 PM Mohammad Izadi <moh.izadi at gmail.com>
> > > wrote:
> > > >
> > > >> From: Mohammad Izadi <izadi at google.com>
> > > >>
> > > >> Trying to read HDR10+ metadata from HEVC/SEI and pass it to packet
> side
> > > >> data in the follow-up CLs.
> > > >> ---
> > > >>  libavutil/hdr_dynamic_metadata.c | 165
> +++++++++++++++++++++++++++++++
> > > >>  libavutil/hdr_dynamic_metadata.h |  13 ++-
> > > >>  libavutil/version.h              |   2 +-
> > > >>  3 files changed, 178 insertions(+), 2 deletions(-)
> > > >>
> > > >> diff --git a/libavutil/hdr_dynamic_metadata.c
> > > >> b/libavutil/hdr_dynamic_metadata.c
> > > >> index 0fa1ee82de..f24bcb40f5 100644
> > > >> --- a/libavutil/hdr_dynamic_metadata.c
> > > >> +++ b/libavutil/hdr_dynamic_metadata.c
> > > >> @@ -19,8 +19,17 @@
> > > >>   */
> > > >>
> > > >>  #include "hdr_dynamic_metadata.h"
> > > >> +#include "libavcodec/get_bits.h"
> > > >>
> > > >
> > > > wait is it ok to use libavcodec headers in libavutil? while it's
> fine at
> > > > compilation time since it is an inlined header, I don't think it's a
> good
> > > > idea to freely use such functions in this way: what I would do is
> rather
> > > > implement the parsing in libavcodec, store the fields in a structure
> > > > defined in libavutil and then use this new structure in here
> > >
> > > This seems excessive only to use a bitstream reader to read a bunch of
> > > fields in a buffer.
> > >
> > > get_bits.h is included in lavf, so i don't see why it couldn't be used
> > > in lavu.
> >
> >
> > bacuase lavf is a library which depends on lavc, not vice versa,
> hierarchy
> > is respected
> >
> > As you said it's inlined.
> > >
> >
> > I still consider it a poor design choice: either make bitstream reader
> > public in a separate library (not easy, i am aware) or do the bitstream
> > reading where the bistream actually is, IMO
> >
>
> I agree that the parsing should just move to avcodec. We parse every
> other SEI straight in the codec it comes from, why does this one have
> parsing in avutil? It seems weird.
>
> - Hendrik
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list