[FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field

Rostislav Pehlivanov atomnuker at gmail.com
Wed May 23 22:25:34 EEST 2018


On 23 May 2018 at 20:01, wm4 <nfxjfg at googlemail.com> wrote:

> On Wed, 23 May 2018 14:29:38 -0400 (EDT)
> Patrick Keroulas <patrick.keroulas at savoirfairelinux.com> wrote:
>
> > ----- Original Message -----
> > > From: "wm4" <nfxjfg at googlemail.com>
> > > To: ffmpeg-devel at ffmpeg.org
> > > Sent: Wednesday, May 23, 2018 12:02:45 PM
> > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for
> packets with top/bottom field
> >
> > > On Wed, 23 May 2018 16:46:17 +0100
> > > Rostislav Pehlivanov <atomnuker at gmail.com> wrote:
> > >
> > >> On 23 May 2018 at 16:18, wm4 <nfxjfg at googlemail.com> wrote:
> > >>
> > >> > On Tue, 22 May 2018 17:19:35 -0400 (EDT)
> > >> > Patrick Keroulas <patrick.keroulas at savoirfairelinux.com> wrote:
> > >> >
> > >> > > ----- Original Message -----
> > >> > > > From: "Rostislav Pehlivanov" <atomnuker at gmail.com>
> > >> > > > To: "FFmpeg development discussions and patches" <
> > >> > ffmpeg-devel at ffmpeg.org>
> > >> > > > Sent: Friday, May 18, 2018 5:28:42 PM
> > >> > > > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags
> for
> > >> > packets with top/bottom field
> > >> > >
> > >> > > > On 18 May 2018 at 22:17, wm4 <nfxjfg at googlemail.com> wrote:
> > >> > >
> > >> > >
> > >> > >
> > >> > > > > But I think a new side data type would be much saner. We
> could even
> > >> > > > > just make it something generic, like AV_PKT_DATA_ANCILLARY or
> > >> > > > > something. It's apparently just packet data which somehow
> couldn't go
> > >> > > > > into the packet data.
> > >> > >
> > >> > >
> > >> > > > I agree, a generic ancillary side data type sounds better. It
> would
> > >> > have to
> > >> > > > be handled the same way as mastering metadata (e.g. to allocate
> it
> > >> > you'd
> > >> > > > need to use a separate function), since the size of the data
> struct
> > >> > can't
> > >> > > > be part of the API if we intend to add fields later.
> > >> > > > Patrick, if you're okay with that you should submit a patch
> which bases
> > >> > > > such side data on libavutil/mastering_display_metadata.h/.c
> > >> > >
> > >> > > No problem for transmitting field flags through side data. But
> the given
> > >> > > example (libavutil/mastering_display_metadata.h/.c) attaches
> data to
> > >> > > AVFrame, not AVPacket, so I'm not sure where to place this
> separate
> > >> > > allocator function. Do you recommend to create a new
> > >> > > libavcodec/ancillary.c/h utility?
> > >> >
> > >> > The example you mentioned exists for AVPacket too (it's just not
> easy
> > >> > to see how it can end up in AVPacket, because no demuxer does that
> > >> > directly).
> > >> >
> > >> > Anyway, ancillary side data would just be an untyped byte array, so
> I
> > >> > don't think it needs any helpers. Just an addition to the packet
> side
> > >> > data enum (I think it's somewhere in avcodec.h).
> > >> > _______________________________________________
> > >> > ffmpeg-devel mailing list
> > >> > ffmpeg-devel at ffmpeg.org
> > >> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >> >
> > >>
> > >> I'd rather have it as a well defined typed array rather than a bunch
> of
> > >> bytes. Otherwise we'd start sending unknown side data info and users
> > >> wouldn't know what to do with it.
> > >
> > > Unless you're adding some meta object system for describing arbitrary
> > > types at runtime I don't know how you'd do that.
> >
> > Is that ok if I simply define a basic struct to hold the field?
> >
> > Any suggestion on where to insert the definition of this data and the
> > accessors in lavc? In a new source file?
>
> If you make it a struct, then make a new file in libavutil, with
> at least a helper to get the struct size (this is for ABI reasons, so
> we can extend the struct later). But then this side data would need a
> specific name, not a generic one like "ancillary".
>
> The display mastering thing is valid for both packets and frames, which
> might be confusing. The thing you add is needed for packets only.
>
> I'd prefer the "ancillary" name and making it just a flat byte array
> instead of a struct and something specific. The former would be like
> extradata, just per packet.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

A flat array would be useless and very codec specific (e.g. if you throw
that side data at one codec it would act in a different way than another
codec), a struct is the way to go here. I don't mind adding another untyped
data if there was a reason, but what we're trying to solve here is very
well defined - determine the field of each packet.

Patrick, like I said, use libavutil/mastering_display_metadata.c/h as a
template.


More information about the ffmpeg-devel mailing list