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

[Patrick Keroulas]

----- Original Message -----
> From: "Rostislav Pehlivanov" <atomnuker at gmail.com>
> To: "FFmpeg development discussions and patches" <ffmpeg-devel at ffmpeg.org>
> Sent: Tuesday, May 15, 2018 4:46:02 PM
> Subject: Re: [FFmpeg-devel] [PATCH v6 1/3] avcodec: add flags for packets with top/bottom field

> On 15 May 2018 at 18:03, wm4 <nfxjfg at googlemail.com> wrote:
> 
>> On Tue, 15 May 2018 17:15:05 +0100
>> Rostislav Pehlivanov <atomnuker at gmail.com> wrote:
>> 
>> > On 15 May 2018 at 15:55, wm4 <nfxjfg at googlemail.com> wrote:
>> > 
>> > > On Mon, 14 May 2018 18:26:35 -0400
>> > > Patrick Keroulas <patrick.keroulas at savoirfairelinux.com> wrote:
>> > > 
>> > > > Signed-off-by: Patrick Keroulas <patrick.keroulas@
>> savoirfairelinux.com>
>> > > > ---
>> > > > doc/APIchanges | 3 +++
>> > > > libavcodec/avcodec.h | 8 ++++++++
>> > > > libavcodec/version.h | 4 ++--
>> > > > 3 files changed, 13 insertions(+), 2 deletions(-)
>> > > > 
>> > > > diff --git a/doc/APIchanges b/doc/APIchanges
>> > > > index bbefc83..d06868e 100644
>> > > > --- a/doc/APIchanges
>> > > > +++ b/doc/APIchanges
>> > > > @@ -15,6 +15,9 @@ libavutil: 2017-10-21
>> > > > 
>> > > > API changes, most recent first:
>> > > > 
>> > > > +2018-05-xx - xxxxxxxxxx - lavc 58.20.100 - avcodec.h
>> > > > + Add AV_PKT_FLAG_TOP_FIELD and AV_PKT_FLAG_BOTTOM_FIELD.
>> > > > +
>> > > > 2018-05-xx - xxxxxxxxxx - lavu 56.18.101 - hwcontext_cuda.h
>> > > > Add AVCUDADeviceContext.stream.
>> > > > 
>> > > > diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
>> > > > index fb0c6fa..14811be 100644
>> > > > --- a/libavcodec/avcodec.h
>> > > > +++ b/libavcodec/avcodec.h
>> > > > @@ -1480,6 +1480,14 @@ typedef struct AVPacket {
>> > > > */
>> > > > #define AV_PKT_FLAG_DISPOSABLE 0x0010
>> > > > 
>> > > > +/**
>> > > > + * The packet contains a top field.
>> > > > + */
>> > > > +#define AV_PKT_FLAG_TOP_FIELD 0x0020
>> > > > +/**
>> > > > + * The packet contains a bottom field.
>> > > > + */
>> > > > +#define AV_PKT_FLAG_BOTTOM_FIELD 0x0040
>> > > > 
>> > > > enum AVSideDataParamChangeFlags {
>> > > > AV_SIDE_DATA_PARAM_CHANGE_CHANNEL_COUNT = 0x0001,
>> > > > diff --git a/libavcodec/version.h b/libavcodec/version.h
>> > > > index 3fda743..b9752ce 100644
>> > > > --- a/libavcodec/version.h
>> > > > +++ b/libavcodec/version.h
>> > > > @@ -28,8 +28,8 @@
>> > > > #include "libavutil/version.h"
>> > > > 
>> > > > #define LIBAVCODEC_VERSION_MAJOR 58
>> > > > -#define LIBAVCODEC_VERSION_MINOR 19
>> > > > -#define LIBAVCODEC_VERSION_MICRO 101
>> > > > +#define LIBAVCODEC_VERSION_MINOR 20
>> > > > +#define LIBAVCODEC_VERSION_MICRO 100
>> > > > 
>> > > > #define LIBAVCODEC_VERSION_INT AV_VERSION_INT(LIBAVCODEC_VERSION_MAJOR,
>> 
>> > > \
>> > > > 
>> > > LIBAVCODEC_VERSION_MINOR, \
>> > > 
>> > > So far we could avoid codec-specific packet flags, and I think it
>> > > should stay this way. Maybe make it side data, something with naming
>> > > specific to the bitpacked codec. Or alternatively, if this codec
>> > > is 100% RTP specific and there's no such thing as standard bitpacked
>> > > packets (e.g. muxed in other files etc.), add it to the packet
>> > > directly. The RTP code "repacks" it already on the libavformat side
>> > > anyway.
>> > > _______________________________________________
>> > > ffmpeg-devel mailing list
>> > > ffmpeg-devel at ffmpeg.org
>> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> > > 
>> > 
>> > This codec isn't RTP specific, its the same as V210. There are no flags
>> in
>> > the bitstream, its just a sequence of packed pixels. And just like v210
>> > there's a standard for what packets need to look like (rfc4175, and
>> > unfortunately it does specify the fields need to be separate), so
>> packing 2
>> > fields in the muxer isn't really an option.
>> 
>> Then why are there separate bitpacked and v210 decoders/codec_ids?
>> 
> 
> Its a different type of packing.
> 
> 
> 
>> > Side data seems a bit of an overkill for a flag. I'd say the field flags
>> > are not codec specific as some raw codecs and containers can signal
>> fields
>> > per packet. So I don't really mind them.
>> 
>> You want codec specific flags to accumulate in AVPacket.flags? Can't way
>> until we change the flags field to int128_t.
>> 
>> 
> No, but I think 2 more bits won't hurt much, especially considering pretty
> much all formats supporting interlaced content split each field into a
> separate packet.

Recomposing a frame from fields on the demux side would make the bitpacked decoder
completely useless. Can we find a consensus? How about reusing AVPictureStructure
as suggested by Derek?

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel