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

Rostislav Pehlivanov atomnuker at gmail.com
Fri May 18 23:59:13 EEST 2018


On 18 May 2018 at 21:03, wm4 <nfxjfg at googlemail.com> wrote:

> On Fri, 18 May 2018 20:09:02 +0100
> Rostislav Pehlivanov <atomnuker at gmail.com> wrote:
>
> > On 18 May 2018 at 20:05, 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: 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
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> >
> > Reusing that structure would mean adding a field to AVPackets. I'd rather
> > avoid that, so I'm okay with the packet flags.
>
> We can't add fields to AVPacket (ABI issues). I'm against the flags
> though. None of the current packet flags are needed for correct
> decoding, why change that suddenly?
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

AV_PKT_FLAG_TRUSTED is needed to decode some packets, so it would not be an
entirely new change.
On the other hand, using side data would mean having to use
AV_CODEC_CAP_PARAM_CHANGE, adding a AV_SIDE_DATA_PARAM_CHANGE_FIELD and
adding a new AVCodecContext field to indicate the current field of a
packet. Or adding a new 1-byte large side data type to indicate packet
field.
I think the packet flag solution is much saner than that.


More information about the ffmpeg-devel mailing list