[FFmpeg-devel] [PATCH v3 2/3] avormat/av1dec: add low-overhead bitstream format
Xu, Guangxin
guangxin.xu at intel.com
Fri Aug 14 05:45:15 EEST 2020
> Should be good with those trivial changes, so i can implement them and push if
> you don't want to send another revision.
>
Great! thanks for you kindly help on this and previous review.
Really appreciate it.
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of James
> Almer
> Sent: Friday, August 14, 2020 3:11 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v3 2/3] avormat/av1dec: add low-
> overhead bitstream format
>
> On 8/13/2020 3:51 AM, Xu Guangxin wrote:
> > Hi James,
> > thanks for your feedback, please help review it again.
> >
> > Changelist for v3:
> > use av_fifo_* instead of homebrewed fifo operations
> > obu_probe(), add padding obu to alllow list
> > read_header(), use "const AVRational* framerate" instead of "AVRational
> framerate"
> >
> >
> >
> > It's defined in Section 5.2, used by netflix.
> > see http://download.opencontent.netflix.com/?prefix=AV1/Chimera/
> > ---
> > configure | 1 +
> > libavformat/allformats.c | 1 +
> > libavformat/av1dec.c | 263 +++++++++++++++++++++++++++++++++++--
> --
> > 3 files changed, 242 insertions(+), 23 deletions(-)
> >
> > diff --git a/configure b/configure
> > index 8de1afcb99..d4a1fea9ce 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3331,6 +3331,7 @@ mxf_d10_muxer_select="mxf_muxer"
> > mxf_opatom_muxer_select="mxf_muxer"
> > nut_muxer_select="riffenc"
> > nuv_demuxer_select="riffdec"
> > +obu_demuxer_select="av1_frame_merge_bsf av1_parser"
> > oga_muxer_select="ogg_muxer"
> > ogg_demuxer_select="dirac_parse"
> > ogv_muxer_select="ogg_muxer"
> > diff --git a/libavformat/allformats.c b/libavformat/allformats.c index
> > b7e59ae170..0aa9dd7198 100644
> > --- a/libavformat/allformats.c
> > +++ b/libavformat/allformats.c
> > @@ -293,6 +293,7 @@ extern AVOutputFormat ff_null_muxer; extern
> > AVInputFormat ff_nut_demuxer; extern AVOutputFormat ff_nut_muxer;
> > extern AVInputFormat ff_nuv_demuxer;
> > +extern AVInputFormat ff_obu_demuxer;
> > extern AVOutputFormat ff_oga_muxer;
> > extern AVInputFormat ff_ogg_demuxer; extern AVOutputFormat
> > ff_ogg_muxer; diff --git a/libavformat/av1dec.c b/libavformat/av1dec.c
> > index 1be2fac1c1..62cf5c31ea 100644
> > --- a/libavformat/av1dec.c
> > +++ b/libavformat/av1dec.c
> > @@ -22,6 +22,7 @@
> > #include "config.h"
> >
> > #include "libavutil/common.h"
> > +#include "libavutil/fifo.h"
> > #include "libavutil/opt.h"
> > #include "libavcodec/av1_parse.h"
> > #include "avformat.h"
> > @@ -70,6 +71,25 @@ static int read_obu(const uint8_t *buf, int size, int64_t
> *obu_size, int *type)
> > return 0;
> > }
> >
> > +//return < 0 if we need more data
> > +static int get_score(int type, int *seq) {
> > + switch (type) {
> > + case AV1_OBU_SEQUENCE_HEADER:
> > + *seq = 1;
> > + return -1;
> > + case AV1_OBU_FRAME:
> > + case AV1_OBU_FRAME_HEADER:
> > + return *seq ? AVPROBE_SCORE_EXTENSION + 1 : 0;
> > + case AV1_OBU_METADATA:
> > + case AV1_OBU_PADDING:
> > + return -1;
> > + default:
> > + break;
> > + }
> > + return 0;
> > +}
> > +
> > static int annexb_probe(const AVProbeData *p) {
> > AVIOContext pb;
> > @@ -123,19 +143,9 @@ static int annexb_probe(const AVProbeData *p)
> > return 0;
> > cnt += obu_unit_size;
> >
> > - switch (type) {
> > - case AV1_OBU_SEQUENCE_HEADER:
> > - seq = 1;
> > - break;
> > - case AV1_OBU_FRAME:
> > - case AV1_OBU_FRAME_HEADER:
> > - return seq ? AVPROBE_SCORE_EXTENSION + 1 : 0;
> > - case AV1_OBU_TILE_GROUP:
> > - case AV1_OBU_TEMPORAL_DELIMITER:
> > - return 0;
> > - default:
> > - break;
> > - }
> > + ret = get_score(type, &seq);
> > + if (ret >= 0)
> > + return ret;
> >
> > temporal_unit_size -= obu_unit_size + ret;
> > frame_unit_size -= obu_unit_size + ret; @@ -144,15 +154,14 @@
> > static int annexb_probe(const AVProbeData *p)
> > return 0;
> > }
> >
> > -static int annexb_read_header(AVFormatContext *s)
> > +static int read_header(AVFormatContext *s, const AVRational
> > +*framerate, AVBSFContext **bsf, void *logctx)
> > {
> > - AnnexBContext *c = s->priv_data;
> > const AVBitStreamFilter *filter =
> av_bsf_get_by_name("av1_frame_merge");
> > AVStream *st;
> > int ret;
> >
> > if (!filter) {
> > - av_log(c, AV_LOG_ERROR, "av1_frame_merge bitstream filter "
> > + av_log(logctx, AV_LOG_ERROR, "av1_frame_merge bitstream filter "
> > "not found. This is a bug, please report it.\n");
> > return AVERROR_BUG;
> > }
> > @@ -165,25 +174,32 @@ static int annexb_read_header(AVFormatContext
> *s)
> > st->codecpar->codec_id = AV_CODEC_ID_AV1;
> > st->need_parsing = AVSTREAM_PARSE_HEADERS;
> >
> > - st->internal->avctx->framerate = c->framerate;
> > + st->internal->avctx->framerate = *framerate;
> > // taken from rawvideo demuxers
> > avpriv_set_pts_info(st, 64, 1, 1200000);
> >
> > - ret = av_bsf_alloc(filter, &c->bsf);
> > + ret = av_bsf_alloc(filter, bsf);
> > if (ret < 0)
> > return ret;
> >
> > - ret = avcodec_parameters_copy(c->bsf->par_in, st->codecpar);
> > + ret = avcodec_parameters_copy((*bsf)->par_in, st->codecpar);
> > if (ret < 0) {
> > - av_bsf_free(&c->bsf);
> > + av_bsf_free(bsf);
> > return ret;
> > }
> >
> > - ret = av_bsf_init(c->bsf);
> > + ret = av_bsf_init(*bsf);
> > if (ret < 0)
> > - av_bsf_free(&c->bsf);
> > + av_bsf_free(bsf);
> >
> > return ret;
> > +
> > +}
> > +
> > +static int annexb_read_header(AVFormatContext *s) {
> > + AnnexBContext *c = s->priv_data;
> > + return read_header(s, &c->framerate, &c->bsf, c);
> > }
> >
> > static int annexb_read_packet(AVFormatContext *s, AVPacket *pkt) @@
> > -251,12 +267,193 @@ static int annexb_read_close(AVFormatContext *s)
> > return 0;
> > }
> >
> > -#define OFFSET(x) offsetof(AnnexBContext, x)
> > +typedef struct ObuContext {
> > + const AVClass *class;
> > + AVBSFContext *bsf;
> > + AVRational framerate;
> > + AVFifoBuffer *fifo;
> > +} ObuContext;
> > +
> > +//For low overhead obu, we can't foresee the obu size before we parsed the
> header.
> > +//So, we can't use parse_obu_header here, since it will check size <=
> > +buf_size //see c27c7b49dc for more details static int
> > +read_obu_with_size(const uint8_t *buf, int buf_size, int64_t
> > +*obu_size, int *type) {
> > + GetBitContext gb;
> > + int ret, extension_flag, start_pos;
> > + int64_t size;
> > +
> > + ret = init_get_bits8(&gb, buf, FFMIN(buf_size, MAX_OBU_HEADER_SIZE));
> > + if (ret < 0)
> > + return ret;
> > +
> > + if (get_bits1(&gb) != 0) // obu_forbidden_bit
> > + return AVERROR_INVALIDDATA;
> > +
> > + *type = get_bits(&gb, 4);
> > + extension_flag = get_bits1(&gb);
> > + if (!get_bits1(&gb)) // has_size_flag
> > + return AVERROR_INVALIDDATA;
> > + skip_bits1(&gb); // obu_reserved_1bit
> > +
> > + if (extension_flag) {
> > + get_bits(&gb, 3); // temporal_id
> > + get_bits(&gb, 2); // spatial_id
> > + skip_bits(&gb, 3); // extension_header_reserved_3bits
> > + }
> > +
> > + *obu_size = leb128(&gb);
> > + if (*obu_size > INT_MAX)
> > + return AVERROR_INVALIDDATA;
> > +
> > + if (get_bits_left(&gb) < 0)
> > + return AVERROR_INVALIDDATA;
> > +
> > + start_pos = get_bits_count(&gb) / 8;
> > +
> > + size = *obu_size + start_pos;
> > + if (size > INT_MAX)
> > + return AVERROR_INVALIDDATA;
> > + return size;
> > +}
> > +
> > +static int obu_probe(const AVProbeData *p) {
> > + int64_t obu_size;
> > + int seq = 0;
> > + int ret, type, cnt;
> > +
> > + // Check that the first OBU is a Temporal Delimiter.
> > + cnt = read_obu_with_size(p->buf, p->buf_size, &obu_size, &type);
> > + if (cnt < 0 || type != AV1_OBU_TEMPORAL_DELIMITER || obu_size != 0)
> > + return 0;
> > +
> > + while (1) {
> > + ret = read_obu_with_size(p->buf + cnt, p->buf_size - cnt, &obu_size,
> &type);
> > + if (ret < 0 || obu_size <= 0)
> > + return 0;
> > + cnt += ret;
> > +
> > + ret = get_score(type, &seq);
> > + if (ret >= 0)
> > + return ret;
> > + }
> > + return 0;
> > +}
> > +
> > +static int obu_read_header(AVFormatContext *s) {
> > + ObuContext *c = s->priv_data;
> > + c->fifo = av_fifo_alloc(MAX_OBU_HEADER_SIZE);
> > + if (!c->fifo)
> > + return AVERROR(ENOMEM);
> > + return read_header(s, &c->framerate, &c->bsf, c); }
> > +
> > +static int obu_prefetch(AVFormatContext *s, uint8_t* dest, int
> > +max_size) {
> > + ObuContext *c = s->priv_data;
> > + int size = max_size - av_fifo_size(c->fifo);
>
> Use av_fifo_space() instead of this for the same effect, and get rid of the
> max_size parameter. You're hardcoding it to MAX_OBU_HEADER_SIZE, so it's
> unnecessary.
>
> > + av_fifo_generic_write(c->fifo, s->pb, size,
> > + (int (*)(void*, void*, int))avio_read);
> > + size = av_fifo_size(c->fifo);
> > + if (size > 0) {
> > + av_fifo_generic_peek(c->fifo, dest, size, NULL);
> > + }
> > + return size;
> > +}
> > +
> > +static int obu_read_data(AVFormatContext *s, AVPacket *pkt, int len)
> > +{
> > + int size, left;
> > + ObuContext *c = s->priv_data;
> > + int ret = av_new_packet(pkt, len);
> > + if (ret < 0) {
> > + av_log(c, AV_LOG_ERROR, "Failed to allocate packet for obu\n");
> > + return ret;
> > + }
> > + size = FFMIN(av_fifo_size(c->fifo), len);
> > + av_fifo_generic_read(c->fifo, pkt->data, size, NULL);
> > + left = len - size;
> > + if (left > 0) {
> > + ret = avio_read(s->pb, pkt->data + size, left);
> > + if (ret != left) {
> > + av_log(c, AV_LOG_ERROR, "Failed to read %d frome file\n", left);
> > + return ret;
>
> This needs to be AVERROR_INVALIDDATA when ret >= 0, otherwise it will not be
> interpreted as an error.
>
> > + }
> > + }
> > + return 0;
> > +}
> > +
> > +static int obu_get_packet(AVFormatContext *s, AVPacket *pkt) {
> > + ObuContext *c = s->priv_data;
> > + int64_t obu_size;
> > + int ret, type;
> > + uint8_t header[MAX_OBU_HEADER_SIZE];
> > +
> > + ret = obu_prefetch(s, header, MAX_OBU_HEADER_SIZE);
> > + if (!ret)
> > + return AVERROR(EOF);
>
> We use AVERROR_EOF rather than AVERROR(EOF) (Afair, it was done because
> EOF is not portable, but don't quote me on it). But in any case, this should return
> 0 in order to let the process call av_bsf_send_packet() with an empty packet,
> which signals EOF and makes the bsf return any potentially buffered packet.
>
> Should be good with those trivial changes, so i can implement them and push if
> you don't want to send another revision.
>
> > +
> > + ret = read_obu_with_size(header, ret, &obu_size, &type);
> > + if (ret < 0) {
> > + av_log(c, AV_LOG_ERROR, "Failed to read obu\n");
> > + return ret;
> > + }
> > + return obu_read_data(s, pkt, ret); }
> > +
> > +static int obu_read_packet(AVFormatContext *s, AVPacket *pkt) {
> > + ObuContext *c = s->priv_data;
> > + int ret;
> > +
> > + while (1) {
> > + ret = obu_get_packet(s, pkt);
> > + if (ret < 0)
> > + return ret;
> > + ret = av_bsf_send_packet(c->bsf, pkt);
> > + if (ret < 0) {
> > + av_log(s, AV_LOG_ERROR, "Failed to send packet to "
> > + "av1_frame_merge filter\n");
> > + return ret;
> > + }
> > + ret = av_bsf_receive_packet(c->bsf, pkt);
> > + if (ret < 0 && ret != AVERROR(EAGAIN) && ret != AVERROR_EOF)
> > + av_log(s, AV_LOG_ERROR, "av1_frame_merge filter failed to "
> > + "send output packet\n");
> > + if (ret != AVERROR(EAGAIN))
> > + break;
> > + }
> > +
> > + return ret;
> > +}
> > +
> > +static int obu_read_close(AVFormatContext *s) {
> > + ObuContext *c = s->priv_data;
> > +
> > + av_fifo_freep(&c->fifo);
> > + av_bsf_free(&c->bsf);
> > + return 0;
> > +}
> > +
> > #define DEC AV_OPT_FLAG_DECODING_PARAM
> > +
> > +#define OFFSET(x) offsetof(AnnexBContext, x)
> > static const AVOption annexb_options[] = {
> > { "framerate", "", OFFSET(framerate), AV_OPT_TYPE_VIDEO_RATE, {.str =
> "25"}, 0, INT_MAX, DEC},
> > { NULL },
> > };
> > +#undef OFFSET
> > +
> > +#define OFFSET(x) offsetof(ObuContext, x) static const AVOption
> > +obu_options[] = {
> > + { "framerate", "", OFFSET(framerate), AV_OPT_TYPE_VIDEO_RATE, {.str =
> "25"}, 0, INT_MAX, DEC},
> > + { NULL },
> > +};
> > +#undef OFFSET
> >
> > static const AVClass annexb_demuxer_class = {
> > .class_name = "AV1 Annex B demuxer", @@ -277,3 +474,23 @@
> > AVInputFormat ff_av1_demuxer = {
> > .flags = AVFMT_GENERIC_INDEX,
> > .priv_class = &annexb_demuxer_class,
> > };
> > +
> > +static const AVClass obu_demuxer_class = {
> > + .class_name = "AV1 low overhead OBU demuxer",
> > + .item_name = av_default_item_name,
> > + .option = obu_options,
> > + .version = LIBAVUTIL_VERSION_INT,
> > +};
> > +
> > +AVInputFormat ff_obu_demuxer = {
> > + .name = "obu",
> > + .long_name = NULL_IF_CONFIG_SMALL("AV1 low overhead OBU"),
> > + .priv_data_size = sizeof(ObuContext),
> > + .read_probe = obu_probe,
> > + .read_header = obu_read_header,
> > + .read_packet = obu_read_packet,
> > + .read_close = obu_read_close,
> > + .extensions = "obu",
> > + .flags = AVFMT_GENERIC_INDEX,
> > + .priv_class = &obu_demuxer_class,
> > +};
> >
>
> _______________________________________________
> 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