----- Original Message ----- > From: "Rostislav Pehlivanov" <atomnu...@gmail.com> > To: "FFmpeg development discussions and patches" <ffmpeg-devel@ffmpeg.org> > Sent: Wednesday, May 23, 2018 3:25:34 PM > Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets > with top/bottom field
> On 23 May 2018 at 20:01, wm4 <nfx...@googlemail.com> wrote: > >> On Wed, 23 May 2018 14:29:38 -0400 (EDT) >> Patrick Keroulas <patrick.kerou...@savoirfairelinux.com> wrote: >> >> > ----- Original Message ----- >> > > From: "wm4" <nfx...@googlemail.com> >> > > To: ffmpeg-devel@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 <atomnu...@gmail.com> wrote: >> > > >> > >> On 23 May 2018 at 16:18, wm4 <nfx...@googlemail.com> wrote: >> > >> >> > >> > On Tue, 22 May 2018 17:19:35 -0400 (EDT) >> > >> > Patrick Keroulas <patrick.kerou...@savoirfairelinux.com> wrote: >> > >> > >> > >> > > ----- Original Message ----- >> > >> > > > From: "Rostislav Pehlivanov" <atomnu...@gmail.com> >> > >> > > > To: "FFmpeg development discussions and patches" < >> > >> > ffmpeg-devel@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 <nfx...@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@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@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. Taking mastering_display_metadata as an example will work for the new struct definition and allocator only. The side_data accessors can't be defined in the same place because there is no concept of AVPacket in libavutil. But they may not be necessary, and using av_packet_*_side_data directly in the demux and the decoder would be fine. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel