diff mbox series

[FFmpeg-devel,4/5] avormat/av1dec: add low-overhead bitstream format

Message ID 20200806080416.17691-5-guangxin.xu@intel.com
State Changes Requested
Headers show
Series avformat/av1dec: add low overhead obu demux | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Xu, Guangxin Aug. 6, 2020, 8:04 a.m. UTC
It's defined in Section 5.2, used by netflix.
see http://download.opencontent.netflix.com/?prefix=AV1/Chimera/
---
 libavformat/allformats.c |   1 +
 libavformat/av1dec.c     | 193 +++++++++++++++++++++++++++++++++++++--
 2 files changed, 185 insertions(+), 9 deletions(-)

Comments

James Almer Aug. 6, 2020, 2:09 p.m. UTC | #1
On 8/6/2020 5:04 AM, Xu Guangxin wrote:
> It's defined in Section 5.2, used by netflix.
> see http://download.opencontent.netflix.com/?prefix=AV1/Chimera/
> ---
>  libavformat/allformats.c |   1 +
>  libavformat/av1dec.c     | 193 +++++++++++++++++++++++++++++++++++++--
>  2 files changed, 185 insertions(+), 9 deletions(-)
> 
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index fd9e46e233..8eead5619d 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -75,6 +75,7 @@ extern AVOutputFormat ff_asf_stream_muxer;
>  extern AVInputFormat  ff_au_demuxer;
>  extern AVOutputFormat ff_au_muxer;
>  extern AVInputFormat  ff_av1_demuxer;
> +extern AVInputFormat  ff_av1_low_overhead_demuxer;

How about ff_obu_demuxer instead?

>  extern AVInputFormat  ff_avi_demuxer;
>  extern AVOutputFormat ff_avi_muxer;
>  extern AVInputFormat  ff_avisynth_demuxer;
> diff --git a/libavformat/av1dec.c b/libavformat/av1dec.c
> index ec66152e03..0c5d172a0f 100644
> --- a/libavformat/av1dec.c
> +++ b/libavformat/av1dec.c
> @@ -28,6 +28,9 @@
>  #include "avio_internal.h"
>  #include "internal.h"
>  
> +//2 + max leb 128 size
> +#define MAX_HEAD_SIZE 10

This value is also used in av1_parse.h, so you could put it there and
reuse it. But if you do, use a less generic name, like MAX_OBU_HEADER_SIZE.

> +
>  typedef struct AnnexBContext {
>      const AVClass *class;
>      AVBSFContext *bsf;
> @@ -139,9 +142,8 @@ static int annexb_probe(const AVProbeData *p)
>      return 0;
>  }
>  
> -static int annexb_read_header(AVFormatContext *s)
> +static int read_header(AVFormatContext *s, AVRational framerate, AVBSFContext **bsf, void *c)

Since c is now a log context, rename it to logctx.

>  {
> -    AnnexBContext *c = s->priv_data;
>      const AVBitStreamFilter *filter = av_bsf_get_by_name("av1_frame_merge");
>      AVStream *st;
>      int ret;
> @@ -160,25 +162,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 it requires this bsf, then it should be added as a depencency to
configure.

>      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)
> @@ -246,12 +255,158 @@ static int annexb_read_close(AVFormatContext *s)
>      return 0;
>  }
>  
> -#define OFFSET(x) offsetof(AnnexBContext, x)
> +typedef struct LowOverheadContext {
> +    const AVClass *class;
> +    AVBSFContext *bsf;
> +    AVRational framerate;
> +    uint8_t prefetched[MAX_HEAD_SIZE];
> +    //prefetched len
> +    int len;
> +} LowOverheadContext;
> +
> +static int low_overhead_probe(const AVProbeData *p)
> +{
> +    AVIOContext pb;
> +    int64_t obu_size;
> +    int ret, type, cnt = 0, has_size_flag;
> +
> +    ffio_init_context(&pb, p->buf, p->buf_size, 0,
> +                      NULL, NULL, NULL, NULL);
> +
> +    // Check that the first OBU is a Temporal Delimiter.
> +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE), &obu_size, &type, &has_size_flag);

cnt is 0 at this point, so might as well remove it.

> +    if (ret < 0 || type != AV1_OBU_TEMPORAL_DELIMITER || obu_size != 0 || !has_size_flag)
> +        return 0;
> +    cnt += ret;
> +
> +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE), &obu_size, &type, &has_size_flag);
> +    if (ret < 0 || type != AV1_OBU_SEQUENCE_HEADER || !has_size_flag)

Afaik, there's nothing in the spec that forbids a Metadata OBU to appear
here before a Sequence Header.

> +        return 0;
> +    cnt += ret;
> +
> +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE), &obu_size, &type, &has_size_flag);
> +    if (ret < 0 || (type != AV1_OBU_FRAME && type != AV1_OBU_FRAME_HEADER) || obu_size <= 0 || !has_size_flag)

Similarly, the third OBU is not required to be a Frame or Frame Header.
It could be Metadata after the second one was a Sequence Header, or
vice-versa.

> +        return 0;
> +    return AVPROBE_SCORE_EXTENSION + 1;
> +}
> +
> +static int low_overhead_read_header(AVFormatContext *s)
> +{
> +    LowOverheadContext *c = s->priv_data;
> +    return read_header(s, c->framerate, &c->bsf, c);
> +}
> +
> +static int low_overhead_prefetch(AVFormatContext *s)
> +{
> +    LowOverheadContext *c = s->priv_data;
> +    int ret;
> +    int size = MAX_HEAD_SIZE - c->len;
> +    if (size > 0 && !avio_feof(s->pb)) {
> +        ret = avio_read(s->pb, &c->prefetched[c->len], size);
> +        if (ret < 0)
> +            return ret;
> +        c->len += ret;
> +    }
> +    return c->len;
> +}
> +
> +static int low_overhead_read_data(AVFormatContext *s, AVPacket *pkt, int size)
> +{
> +    int left;
> +    LowOverheadContext *c = s->priv_data;
> +    int ret = av_new_packet(pkt, size);
> +    if (ret < 0) {
> +        av_log(c, AV_LOG_ERROR, "Failed to allocate packet for obu\n");
> +        return ret;
> +    }
> +    if (size <= c->len) {
> +        memcpy(pkt->data, c->prefetched, size);
> +
> +        left = c->len - size;
> +        memcpy(c->prefetched, c->prefetched + size, left);

This looks like it should be a memmove, as it can overlap.

> +        c->len = left;
> +    } else {
> +        memcpy(pkt->data, c->prefetched, c->len);
> +        left = size - c->len;
> +        ret = avio_read(s->pb, pkt->data + c->len, left);
> +        if (ret != left) {
> +            av_log(c, AV_LOG_ERROR, "Failed to read %d frome file\n", left);
> +            return ret;
> +        }
> +        c->len = 0;
> +    }
> +    return 0;
> +}
> +
> +static int low_overhead_get_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    int64_t obu_size;
> +    int ret, type, has_size_flag;
> +    LowOverheadContext *c = s->priv_data;
> +    ret = low_overhead_prefetch(s);
> +    if (ret < 0)
> +        return ret;
> +    if (ret) {
> +        ret = read_obu(c->prefetched, c->len, &obu_size, &type, &has_size_flag);
> +        if (ret < 0 && !has_size_flag) {

Shouldn't this be (ret < 0 || !has_size_flag)?

> +            av_log(c, AV_LOG_ERROR, "Failed to read obu\n");
> +            return ret;
> +        }
> +        ret = low_overhead_read_data(s, pkt, ret);
> +    }
> +    return ret;
> +}
> +
> +static int low_overhead_read_packet(AVFormatContext *s, AVPacket *pkt)
> +{
> +    LowOverheadContext *c = s->priv_data;
> +    int ret;
> +
> +    while (1) {
> +        ret = low_overhead_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 low_overhead_read_close(AVFormatContext *s)
> +{
> +    LowOverheadContext *c = s->priv_data;
> +
> +    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(LowOverheadContext, x)
> +static const AVOption low_overhead_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",
> @@ -272,3 +427,23 @@ AVInputFormat ff_av1_demuxer = {
>      .flags          = AVFMT_GENERIC_INDEX,
>      .priv_class     = &annexb_demuxer_class,
>  };
> +
> +static const AVClass low_overhead_demuxer_class = {
> +    .class_name = "AV1 low overhead OBU demuxer",
> +    .item_name  = av_default_item_name,
> +    .option     = low_overhead_options,
> +    .version    = LIBAVUTIL_VERSION_INT,
> +};
> +
> +AVInputFormat ff_av1_low_overhead_demuxer = {
> +    .name           = "av1",

This name is already used by the AnnexB demuxer, so change it to "obu".

> +    .long_name      = NULL_IF_CONFIG_SMALL("AV1 low overhead OBU"),
> +    .priv_data_size = sizeof(LowOverheadContext),
> +    .read_probe     = low_overhead_probe,
> +    .read_header    = low_overhead_read_header,
> +    .read_packet    = low_overhead_read_packet,
> +    .read_close     = low_overhead_read_close,
> +    .extensions     = "obu",
> +    .flags          = AVFMT_GENERIC_INDEX,
> +    .priv_class     = &low_overhead_demuxer_class,
> +};
>
Xu, Guangxin Aug. 6, 2020, 3:34 p.m. UTC | #2
Hi James,
Comment inline.

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James
> Almer
> Sent: Thursday, August 6, 2020 10:09 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 4/5] avormat/av1dec: add low-overhead
> bitstream format
> 
> On 8/6/2020 5:04 AM, Xu Guangxin wrote:
> > It's defined in Section 5.2, used by netflix.
> > see http://download.opencontent.netflix.com/?prefix=AV1/Chimera/
> > ---
> >  libavformat/allformats.c |   1 +
> >  libavformat/av1dec.c     | 193
> +++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 185 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavformat/allformats.c b/libavformat/allformats.c index
> > fd9e46e233..8eead5619d 100644
> > --- a/libavformat/allformats.c
> > +++ b/libavformat/allformats.c
> > @@ -75,6 +75,7 @@ extern AVOutputFormat ff_asf_stream_muxer;  extern
> > AVInputFormat  ff_au_demuxer;  extern AVOutputFormat ff_au_muxer;
> > extern AVInputFormat  ff_av1_demuxer;
> > +extern AVInputFormat  ff_av1_low_overhead_demuxer;
> 
> How about ff_obu_demuxer instead?
You have a good taste:).
Yes the original name is ff_obu_demuxer, I change it to this before the I send to review.
I will change it back. I guess you also suggest I change all low_overhead things in av1dec.c to obu. Right?

> 
> >  extern AVInputFormat  ff_avi_demuxer;  extern AVOutputFormat
> > ff_avi_muxer;  extern AVInputFormat  ff_avisynth_demuxer; diff --git
> > a/libavformat/av1dec.c b/libavformat/av1dec.c index
> > ec66152e03..0c5d172a0f 100644
> > --- a/libavformat/av1dec.c
> > +++ b/libavformat/av1dec.c
> > @@ -28,6 +28,9 @@
> >  #include "avio_internal.h"
> >  #include "internal.h"
> >
> > +//2 + max leb 128 size
> > +#define MAX_HEAD_SIZE 10
> 
> This value is also used in av1_parse.h, so you could put it there and reuse it. But
> if you do, use a less generic name, like MAX_OBU_HEADER_SIZE.
> 
Sure. 

> > +
> >  typedef struct AnnexBContext {
> >      const AVClass *class;
> >      AVBSFContext *bsf;
> > @@ -139,9 +142,8 @@ static int annexb_probe(const AVProbeData *p)
> >      return 0;
> >  }
> >
> > -static int annexb_read_header(AVFormatContext *s)
> > +static int read_header(AVFormatContext *s, AVRational framerate,
> > +AVBSFContext **bsf, void *c)
> 
> Since c is now a log context, rename it to logctx.
> 
Sure.


> >  {
> > -    AnnexBContext *c = s->priv_data;
> >      const AVBitStreamFilter *filter =
> av_bsf_get_by_name("av1_frame_merge");
> >      AVStream *st;
> >      int ret;
> > @@ -160,25 +162,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 it requires this bsf, then it should be added as a depencency to configure.
> 
> >      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) @@
> > -246,12 +255,158 @@ static int annexb_read_close(AVFormatContext *s)
> >      return 0;
> >  }
> >
> > -#define OFFSET(x) offsetof(AnnexBContext, x)
> > +typedef struct LowOverheadContext {
> > +    const AVClass *class;
> > +    AVBSFContext *bsf;
> > +    AVRational framerate;
> > +    uint8_t prefetched[MAX_HEAD_SIZE];
> > +    //prefetched len
> > +    int len;
> > +} LowOverheadContext;
> > +
> > +static int low_overhead_probe(const AVProbeData *p) {
> > +    AVIOContext pb;
> > +    int64_t obu_size;
> > +    int ret, type, cnt = 0, has_size_flag;
> > +
> > +    ffio_init_context(&pb, p->buf, p->buf_size, 0,
> > +                      NULL, NULL, NULL, NULL);
> > +
> > +    // Check that the first OBU is a Temporal Delimiter.
> > +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt,
> > + MAX_HEAD_SIZE), &obu_size, &type, &has_size_flag);
> 
> cnt is 0 at this point, so might as well remove it.
Accept.

> 
> > +    if (ret < 0 || type != AV1_OBU_TEMPORAL_DELIMITER || obu_size != 0
> || !has_size_flag)
> > +        return 0;
> > +    cnt += ret;
> > +
> > +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE),
> &obu_size, &type, &has_size_flag);
> > +    if (ret < 0 || type != AV1_OBU_SEQUENCE_HEADER || !has_size_flag)
> 
> Afaik, there's nothing in the spec that forbids a Metadata OBU to appear here
> before a Sequence Header.
> 
You are right.

> > +        return 0;
> > +    cnt += ret;
> > +
> > +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE),
> &obu_size, &type, &has_size_flag);
> > +    if (ret < 0 || (type != AV1_OBU_FRAME && type !=
> > + AV1_OBU_FRAME_HEADER) || obu_size <= 0 || !has_size_flag)
> 
> Similarly, the third OBU is not required to be a Frame or Frame Header.
> It could be Metadata after the second one was a Sequence Header, or vice-versa.
> 
Will change

> > +        return 0;
> > +    return AVPROBE_SCORE_EXTENSION + 1; }
> > +
> > +static int low_overhead_read_header(AVFormatContext *s) {
> > +    LowOverheadContext *c = s->priv_data;
> > +    return read_header(s, c->framerate, &c->bsf, c); }
> > +
> > +static int low_overhead_prefetch(AVFormatContext *s) {
> > +    LowOverheadContext *c = s->priv_data;
> > +    int ret;
> > +    int size = MAX_HEAD_SIZE - c->len;
> > +    if (size > 0 && !avio_feof(s->pb)) {
> > +        ret = avio_read(s->pb, &c->prefetched[c->len], size);
> > +        if (ret < 0)
> > +            return ret;
> > +        c->len += ret;
> > +    }
> > +    return c->len;
> > +}
> > +
> > +static int low_overhead_read_data(AVFormatContext *s, AVPacket *pkt,
> > +int size) {
> > +    int left;
> > +    LowOverheadContext *c = s->priv_data;
> > +    int ret = av_new_packet(pkt, size);
> > +    if (ret < 0) {
> > +        av_log(c, AV_LOG_ERROR, "Failed to allocate packet for obu\n");
> > +        return ret;
> > +    }
> > +    if (size <= c->len) {
> > +        memcpy(pkt->data, c->prefetched, size);
> > +
> > +        left = c->len - size;
> > +        memcpy(c->prefetched, c->prefetched + size, left);
> 
> This looks like it should be a memmove, as it can overlap.
Memmove is same thing as memcpy if dest <= src
See https://elixir.bootlin.com/linux/v4.3/source/lib/string.c#L734

> 
> > +        c->len = left;
> > +    } else {
> > +        memcpy(pkt->data, c->prefetched, c->len);
> > +        left = size - c->len;
> > +        ret = avio_read(s->pb, pkt->data + c->len, left);
> > +        if (ret != left) {
> > +            av_log(c, AV_LOG_ERROR, "Failed to read %d frome file\n", left);
> > +            return ret;
> > +        }
> > +        c->len = 0;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int low_overhead_get_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > +    int64_t obu_size;
> > +    int ret, type, has_size_flag;
> > +    LowOverheadContext *c = s->priv_data;
> > +    ret = low_overhead_prefetch(s);
> > +    if (ret < 0)
> > +        return ret;
> > +    if (ret) {
> > +        ret = read_obu(c->prefetched, c->len, &obu_size, &type,
> &has_size_flag);
> > +        if (ret < 0 && !has_size_flag) {
> 
> Shouldn't this be (ret < 0 || !has_size_flag)?
Yes you are right, will change.


> 
> > +            av_log(c, AV_LOG_ERROR, "Failed to read obu\n");
> > +            return ret;
> > +        }
> > +        ret = low_overhead_read_data(s, pkt, ret);
> > +    }
> > +    return ret;
> > +}
> > +
> > +static int low_overhead_read_packet(AVFormatContext *s, AVPacket
> > +*pkt) {
> > +    LowOverheadContext *c = s->priv_data;
> > +    int ret;
> > +
> > +    while (1) {
> > +        ret = low_overhead_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 low_overhead_read_close(AVFormatContext *s) {
> > +    LowOverheadContext *c = s->priv_data;
> > +
> > +    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(LowOverheadContext, x) static const
> > +AVOption low_overhead_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", @@ -272,3 +427,23 @@
> > AVInputFormat ff_av1_demuxer = {
> >      .flags          = AVFMT_GENERIC_INDEX,
> >      .priv_class     = &annexb_demuxer_class,
> >  };
> > +
> > +static const AVClass low_overhead_demuxer_class = {
> > +    .class_name = "AV1 low overhead OBU demuxer",
> > +    .item_name  = av_default_item_name,
> > +    .option     = low_overhead_options,
> > +    .version    = LIBAVUTIL_VERSION_INT,
> > +};
> > +
> > +AVInputFormat ff_av1_low_overhead_demuxer = {
> > +    .name           = "av1",
> 
> This name is already used by the AnnexB demuxer, so change it to "obu".
Sure.

> 
> > +    .long_name      = NULL_IF_CONFIG_SMALL("AV1 low overhead OBU"),
> > +    .priv_data_size = sizeof(LowOverheadContext),
> > +    .read_probe     = low_overhead_probe,
> > +    .read_header    = low_overhead_read_header,
> > +    .read_packet    = low_overhead_read_packet,
> > +    .read_close     = low_overhead_read_close,
> > +    .extensions     = "obu",
> > +    .flags          = AVFMT_GENERIC_INDEX,
> > +    .priv_class     = &low_overhead_demuxer_class,
> > +};
> >
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
Xu, Guangxin Aug. 6, 2020, 3:40 p.m. UTC | #3
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James
> Almer
> Sent: Thursday, August 6, 2020 10:09 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 4/5] avormat/av1dec: add low-overhead
> bitstream format
> 
> On 8/6/2020 5:04 AM, Xu Guangxin wrote:
> > It's defined in Section 5.2, used by netflix.
> > see http://download.opencontent.netflix.com/?prefix=AV1/Chimera/
> > ---
> >  libavformat/allformats.c |   1 +
> >  libavformat/av1dec.c     | 193
> +++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 185 insertions(+), 9 deletions(-)
> >
> > diff --git a/libavformat/allformats.c b/libavformat/allformats.c index
> > fd9e46e233..8eead5619d 100644
> > --- a/libavformat/allformats.c
> > +++ b/libavformat/allformats.c
> > @@ -75,6 +75,7 @@ extern AVOutputFormat ff_asf_stream_muxer;  extern
> > AVInputFormat  ff_au_demuxer;  extern AVOutputFormat ff_au_muxer;
> > extern AVInputFormat  ff_av1_demuxer;
> > +extern AVInputFormat  ff_av1_low_overhead_demuxer;
> 
> How about ff_obu_demuxer instead?
> 
> >  extern AVInputFormat  ff_avi_demuxer;  extern AVOutputFormat
> > ff_avi_muxer;  extern AVInputFormat  ff_avisynth_demuxer; diff --git
> > a/libavformat/av1dec.c b/libavformat/av1dec.c index
> > ec66152e03..0c5d172a0f 100644
> > --- a/libavformat/av1dec.c
> > +++ b/libavformat/av1dec.c
> > @@ -28,6 +28,9 @@
> >  #include "avio_internal.h"
> >  #include "internal.h"
> >
> > +//2 + max leb 128 size
> > +#define MAX_HEAD_SIZE 10
> 
> This value is also used in av1_parse.h, so you could put it there and reuse it. But
> if you do, use a less generic name, like MAX_OBU_HEADER_SIZE.
> 
> > +
> >  typedef struct AnnexBContext {
> >      const AVClass *class;
> >      AVBSFContext *bsf;
> > @@ -139,9 +142,8 @@ static int annexb_probe(const AVProbeData *p)
> >      return 0;
> >  }
> >
> > -static int annexb_read_header(AVFormatContext *s)
> > +static int read_header(AVFormatContext *s, AVRational framerate,
> > +AVBSFContext **bsf, void *c)
> 
> Since c is now a log context, rename it to logctx.
> 
> >  {
> > -    AnnexBContext *c = s->priv_data;
> >      const AVBitStreamFilter *filter =
> av_bsf_get_by_name("av1_frame_merge");
> >      AVStream *st;
> >      int ret;
> > @@ -160,25 +162,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 it requires this bsf, then it should be added as a depencency to configure.
Could you explain more about this.
The entire function is from original code, I just changed some signature.
thanks

> 
> >      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) @@
> > -246,12 +255,158 @@ static int annexb_read_close(AVFormatContext *s)
> >      return 0;
> >  }
> >
> > -#define OFFSET(x) offsetof(AnnexBContext, x)
> > +typedef struct LowOverheadContext {
> > +    const AVClass *class;
> > +    AVBSFContext *bsf;
> > +    AVRational framerate;
> > +    uint8_t prefetched[MAX_HEAD_SIZE];
> > +    //prefetched len
> > +    int len;
> > +} LowOverheadContext;
> > +
> > +static int low_overhead_probe(const AVProbeData *p) {
> > +    AVIOContext pb;
> > +    int64_t obu_size;
> > +    int ret, type, cnt = 0, has_size_flag;
> > +
> > +    ffio_init_context(&pb, p->buf, p->buf_size, 0,
> > +                      NULL, NULL, NULL, NULL);
> > +
> > +    // Check that the first OBU is a Temporal Delimiter.
> > +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt,
> > + MAX_HEAD_SIZE), &obu_size, &type, &has_size_flag);
> 
> cnt is 0 at this point, so might as well remove it.
> 
> > +    if (ret < 0 || type != AV1_OBU_TEMPORAL_DELIMITER || obu_size != 0
> || !has_size_flag)
> > +        return 0;
> > +    cnt += ret;
> > +
> > +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE),
> &obu_size, &type, &has_size_flag);
> > +    if (ret < 0 || type != AV1_OBU_SEQUENCE_HEADER || !has_size_flag)
> 
> Afaik, there's nothing in the spec that forbids a Metadata OBU to appear here
> before a Sequence Header.
> 
> > +        return 0;
> > +    cnt += ret;
> > +
> > +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE),
> &obu_size, &type, &has_size_flag);
> > +    if (ret < 0 || (type != AV1_OBU_FRAME && type !=
> > + AV1_OBU_FRAME_HEADER) || obu_size <= 0 || !has_size_flag)
> 
> Similarly, the third OBU is not required to be a Frame or Frame Header.
> It could be Metadata after the second one was a Sequence Header, or vice-versa.
> 
> > +        return 0;
> > +    return AVPROBE_SCORE_EXTENSION + 1; }
> > +
> > +static int low_overhead_read_header(AVFormatContext *s) {
> > +    LowOverheadContext *c = s->priv_data;
> > +    return read_header(s, c->framerate, &c->bsf, c); }
> > +
> > +static int low_overhead_prefetch(AVFormatContext *s) {
> > +    LowOverheadContext *c = s->priv_data;
> > +    int ret;
> > +    int size = MAX_HEAD_SIZE - c->len;
> > +    if (size > 0 && !avio_feof(s->pb)) {
> > +        ret = avio_read(s->pb, &c->prefetched[c->len], size);
> > +        if (ret < 0)
> > +            return ret;
> > +        c->len += ret;
> > +    }
> > +    return c->len;
> > +}
> > +
> > +static int low_overhead_read_data(AVFormatContext *s, AVPacket *pkt,
> > +int size) {
> > +    int left;
> > +    LowOverheadContext *c = s->priv_data;
> > +    int ret = av_new_packet(pkt, size);
> > +    if (ret < 0) {
> > +        av_log(c, AV_LOG_ERROR, "Failed to allocate packet for obu\n");
> > +        return ret;
> > +    }
> > +    if (size <= c->len) {
> > +        memcpy(pkt->data, c->prefetched, size);
> > +
> > +        left = c->len - size;
> > +        memcpy(c->prefetched, c->prefetched + size, left);
> 
> This looks like it should be a memmove, as it can overlap.
> 
> > +        c->len = left;
> > +    } else {
> > +        memcpy(pkt->data, c->prefetched, c->len);
> > +        left = size - c->len;
> > +        ret = avio_read(s->pb, pkt->data + c->len, left);
> > +        if (ret != left) {
> > +            av_log(c, AV_LOG_ERROR, "Failed to read %d frome file\n", left);
> > +            return ret;
> > +        }
> > +        c->len = 0;
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int low_overhead_get_packet(AVFormatContext *s, AVPacket *pkt)
> > +{
> > +    int64_t obu_size;
> > +    int ret, type, has_size_flag;
> > +    LowOverheadContext *c = s->priv_data;
> > +    ret = low_overhead_prefetch(s);
> > +    if (ret < 0)
> > +        return ret;
> > +    if (ret) {
> > +        ret = read_obu(c->prefetched, c->len, &obu_size, &type,
> &has_size_flag);
> > +        if (ret < 0 && !has_size_flag) {
> 
> Shouldn't this be (ret < 0 || !has_size_flag)?
> 
> > +            av_log(c, AV_LOG_ERROR, "Failed to read obu\n");
> > +            return ret;
> > +        }
> > +        ret = low_overhead_read_data(s, pkt, ret);
> > +    }
> > +    return ret;
> > +}
> > +
> > +static int low_overhead_read_packet(AVFormatContext *s, AVPacket
> > +*pkt) {
> > +    LowOverheadContext *c = s->priv_data;
> > +    int ret;
> > +
> > +    while (1) {
> > +        ret = low_overhead_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 low_overhead_read_close(AVFormatContext *s) {
> > +    LowOverheadContext *c = s->priv_data;
> > +
> > +    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(LowOverheadContext, x) static const
> > +AVOption low_overhead_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", @@ -272,3 +427,23 @@
> > AVInputFormat ff_av1_demuxer = {
> >      .flags          = AVFMT_GENERIC_INDEX,
> >      .priv_class     = &annexb_demuxer_class,
> >  };
> > +
> > +static const AVClass low_overhead_demuxer_class = {
> > +    .class_name = "AV1 low overhead OBU demuxer",
> > +    .item_name  = av_default_item_name,
> > +    .option     = low_overhead_options,
> > +    .version    = LIBAVUTIL_VERSION_INT,
> > +};
> > +
> > +AVInputFormat ff_av1_low_overhead_demuxer = {
> > +    .name           = "av1",
> 
> This name is already used by the AnnexB demuxer, so change it to "obu".
> 
> > +    .long_name      = NULL_IF_CONFIG_SMALL("AV1 low overhead OBU"),
> > +    .priv_data_size = sizeof(LowOverheadContext),
> > +    .read_probe     = low_overhead_probe,
> > +    .read_header    = low_overhead_read_header,
> > +    .read_packet    = low_overhead_read_packet,
> > +    .read_close     = low_overhead_read_close,
> > +    .extensions     = "obu",
> > +    .flags          = AVFMT_GENERIC_INDEX,
> > +    .priv_class     = &low_overhead_demuxer_class,
> > +};
> >
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
> with subject "unsubscribe".
James Almer Aug. 6, 2020, 5:39 p.m. UTC | #4
On 8/6/2020 12:40 PM, Xu, Guangxin wrote:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James
>> Almer
>> Sent: Thursday, August 6, 2020 10:09 PM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH 4/5] avormat/av1dec: add low-overhead
>> bitstream format
>>
>> On 8/6/2020 5:04 AM, Xu Guangxin wrote:
>>> It's defined in Section 5.2, used by netflix.
>>> see http://download.opencontent.netflix.com/?prefix=AV1/Chimera/
>>> ---
>>>  libavformat/allformats.c |   1 +
>>>  libavformat/av1dec.c     | 193
>> +++++++++++++++++++++++++++++++++++++--
>>>  2 files changed, 185 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/libavformat/allformats.c b/libavformat/allformats.c index
>>> fd9e46e233..8eead5619d 100644
>>> --- a/libavformat/allformats.c
>>> +++ b/libavformat/allformats.c
>>> @@ -75,6 +75,7 @@ extern AVOutputFormat ff_asf_stream_muxer;  extern
>>> AVInputFormat  ff_au_demuxer;  extern AVOutputFormat ff_au_muxer;
>>> extern AVInputFormat  ff_av1_demuxer;
>>> +extern AVInputFormat  ff_av1_low_overhead_demuxer;
>>
>> How about ff_obu_demuxer instead?
>>
>>>  extern AVInputFormat  ff_avi_demuxer;  extern AVOutputFormat
>>> ff_avi_muxer;  extern AVInputFormat  ff_avisynth_demuxer; diff --git
>>> a/libavformat/av1dec.c b/libavformat/av1dec.c index
>>> ec66152e03..0c5d172a0f 100644
>>> --- a/libavformat/av1dec.c
>>> +++ b/libavformat/av1dec.c
>>> @@ -28,6 +28,9 @@
>>>  #include "avio_internal.h"
>>>  #include "internal.h"
>>>
>>> +//2 + max leb 128 size
>>> +#define MAX_HEAD_SIZE 10
>>
>> This value is also used in av1_parse.h, so you could put it there and reuse it. But
>> if you do, use a less generic name, like MAX_OBU_HEADER_SIZE.
>>
>>> +
>>>  typedef struct AnnexBContext {
>>>      const AVClass *class;
>>>      AVBSFContext *bsf;
>>> @@ -139,9 +142,8 @@ static int annexb_probe(const AVProbeData *p)
>>>      return 0;
>>>  }
>>>
>>> -static int annexb_read_header(AVFormatContext *s)
>>> +static int read_header(AVFormatContext *s, AVRational framerate,
>>> +AVBSFContext **bsf, void *c)
>>
>> Since c is now a log context, rename it to logctx.
>>
>>>  {
>>> -    AnnexBContext *c = s->priv_data;
>>>      const AVBitStreamFilter *filter =
>> av_bsf_get_by_name("av1_frame_merge");
>>>      AVStream *st;
>>>      int ret;
>>> @@ -160,25 +162,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 it requires this bsf, then it should be added as a depencency to configure.
> Could you explain more about this.
> The entire function is from original code, I just changed some signature.
> thanks

You're using the av1_frame_merge bitstream filter to assemble temporal
units. This means this demuxer depends on its presence to work.
This needs to be signaled in configure so, in any build where this
demuxer is present, the bsf will also be present.
For the annexb demuxer, this was done with the line:

av1_demuxer_select="av1_frame_merge_bsf av1_parser"

Which also ensures the av1 parser is present. For this new demuxer,
you'd do:

obu_demuxer_select="av1_frame_merge_bsf av1_parser"

(Assuming you change the name like i suggested).
James Almer Aug. 7, 2020, 3:05 a.m. UTC | #5
On 8/6/2020 12:34 PM, Xu, Guangxin wrote:
> Hi James,
> Comment inline.
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of James
>> Almer
>> Sent: Thursday, August 6, 2020 10:09 PM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH 4/5] avormat/av1dec: add low-overhead
>> bitstream format
>>
>> On 8/6/2020 5:04 AM, Xu Guangxin wrote:
>>> It's defined in Section 5.2, used by netflix.
>>> see http://download.opencontent.netflix.com/?prefix=AV1/Chimera/
>>> ---
>>>  libavformat/allformats.c |   1 +
>>>  libavformat/av1dec.c     | 193
>> +++++++++++++++++++++++++++++++++++++--
>>>  2 files changed, 185 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/libavformat/allformats.c b/libavformat/allformats.c index
>>> fd9e46e233..8eead5619d 100644
>>> --- a/libavformat/allformats.c
>>> +++ b/libavformat/allformats.c
>>> @@ -75,6 +75,7 @@ extern AVOutputFormat ff_asf_stream_muxer;  extern
>>> AVInputFormat  ff_au_demuxer;  extern AVOutputFormat ff_au_muxer;
>>> extern AVInputFormat  ff_av1_demuxer;
>>> +extern AVInputFormat  ff_av1_low_overhead_demuxer;
>>
>> How about ff_obu_demuxer instead?
> You have a good taste:).
> Yes the original name is ff_obu_demuxer, I change it to this before the I send to review.
> I will change it back. I guess you also suggest I change all low_overhead things in av1dec.c to obu. Right?
> 
>>
>>>  extern AVInputFormat  ff_avi_demuxer;  extern AVOutputFormat
>>> ff_avi_muxer;  extern AVInputFormat  ff_avisynth_demuxer; diff --git
>>> a/libavformat/av1dec.c b/libavformat/av1dec.c index
>>> ec66152e03..0c5d172a0f 100644
>>> --- a/libavformat/av1dec.c
>>> +++ b/libavformat/av1dec.c
>>> @@ -28,6 +28,9 @@
>>>  #include "avio_internal.h"
>>>  #include "internal.h"
>>>
>>> +//2 + max leb 128 size
>>> +#define MAX_HEAD_SIZE 10
>>
>> This value is also used in av1_parse.h, so you could put it there and reuse it. But
>> if you do, use a less generic name, like MAX_OBU_HEADER_SIZE.
>>
> Sure. 
> 
>>> +
>>>  typedef struct AnnexBContext {
>>>      const AVClass *class;
>>>      AVBSFContext *bsf;
>>> @@ -139,9 +142,8 @@ static int annexb_probe(const AVProbeData *p)
>>>      return 0;
>>>  }
>>>
>>> -static int annexb_read_header(AVFormatContext *s)
>>> +static int read_header(AVFormatContext *s, AVRational framerate,
>>> +AVBSFContext **bsf, void *c)
>>
>> Since c is now a log context, rename it to logctx.
>>
> Sure.
> 
> 
>>>  {
>>> -    AnnexBContext *c = s->priv_data;
>>>      const AVBitStreamFilter *filter =
>> av_bsf_get_by_name("av1_frame_merge");
>>>      AVStream *st;
>>>      int ret;
>>> @@ -160,25 +162,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 it requires this bsf, then it should be added as a depencency to configure.
>>
>>>      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) @@
>>> -246,12 +255,158 @@ static int annexb_read_close(AVFormatContext *s)
>>>      return 0;
>>>  }
>>>
>>> -#define OFFSET(x) offsetof(AnnexBContext, x)
>>> +typedef struct LowOverheadContext {
>>> +    const AVClass *class;
>>> +    AVBSFContext *bsf;
>>> +    AVRational framerate;
>>> +    uint8_t prefetched[MAX_HEAD_SIZE];
>>> +    //prefetched len
>>> +    int len;
>>> +} LowOverheadContext;
>>> +
>>> +static int low_overhead_probe(const AVProbeData *p) {
>>> +    AVIOContext pb;
>>> +    int64_t obu_size;
>>> +    int ret, type, cnt = 0, has_size_flag;
>>> +
>>> +    ffio_init_context(&pb, p->buf, p->buf_size, 0,
>>> +                      NULL, NULL, NULL, NULL);
>>> +
>>> +    // Check that the first OBU is a Temporal Delimiter.
>>> +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt,
>>> + MAX_HEAD_SIZE), &obu_size, &type, &has_size_flag);
>>
>> cnt is 0 at this point, so might as well remove it.
> Accept.
> 
>>
>>> +    if (ret < 0 || type != AV1_OBU_TEMPORAL_DELIMITER || obu_size != 0
>> || !has_size_flag)
>>> +        return 0;
>>> +    cnt += ret;
>>> +
>>> +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE),
>> &obu_size, &type, &has_size_flag);
>>> +    if (ret < 0 || type != AV1_OBU_SEQUENCE_HEADER || !has_size_flag)
>>
>> Afaik, there's nothing in the spec that forbids a Metadata OBU to appear here
>> before a Sequence Header.
>>
> You are right.
> 
>>> +        return 0;
>>> +    cnt += ret;
>>> +
>>> +    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE),
>> &obu_size, &type, &has_size_flag);
>>> +    if (ret < 0 || (type != AV1_OBU_FRAME && type !=
>>> + AV1_OBU_FRAME_HEADER) || obu_size <= 0 || !has_size_flag)
>>
>> Similarly, the third OBU is not required to be a Frame or Frame Header.
>> It could be Metadata after the second one was a Sequence Header, or vice-versa.
>>
> Will change
> 
>>> +        return 0;
>>> +    return AVPROBE_SCORE_EXTENSION + 1; }
>>> +
>>> +static int low_overhead_read_header(AVFormatContext *s) {
>>> +    LowOverheadContext *c = s->priv_data;
>>> +    return read_header(s, c->framerate, &c->bsf, c); }
>>> +
>>> +static int low_overhead_prefetch(AVFormatContext *s) {
>>> +    LowOverheadContext *c = s->priv_data;
>>> +    int ret;
>>> +    int size = MAX_HEAD_SIZE - c->len;
>>> +    if (size > 0 && !avio_feof(s->pb)) {
>>> +        ret = avio_read(s->pb, &c->prefetched[c->len], size);
>>> +        if (ret < 0)
>>> +            return ret;
>>> +        c->len += ret;
>>> +    }
>>> +    return c->len;
>>> +}
>>> +
>>> +static int low_overhead_read_data(AVFormatContext *s, AVPacket *pkt,
>>> +int size) {
>>> +    int left;
>>> +    LowOverheadContext *c = s->priv_data;
>>> +    int ret = av_new_packet(pkt, size);
>>> +    if (ret < 0) {
>>> +        av_log(c, AV_LOG_ERROR, "Failed to allocate packet for obu\n");
>>> +        return ret;
>>> +    }
>>> +    if (size <= c->len) {
>>> +        memcpy(pkt->data, c->prefetched, size);
>>> +
>>> +        left = c->len - size;
>>> +        memcpy(c->prefetched, c->prefetched + size, left);
>>
>> This looks like it should be a memmove, as it can overlap.
> Memmove is same thing as memcpy if dest <= src
> See https://elixir.bootlin.com/linux/v4.3/source/lib/string.c#L734

https://man7.org/linux/man-pages/man3/memcpy.3.html strictly states
memory areas must not overlap. Even that implemention you linked says as
much, so we can't use memcpy() here.

A single implementation of a standard function isn't representative of
every other implementation.

> 
>>
>>> +        c->len = left;
>>> +    } else {
>>> +        memcpy(pkt->data, c->prefetched, c->len);
>>> +        left = size - c->len;
>>> +        ret = avio_read(s->pb, pkt->data + c->len, left);
>>> +        if (ret != left) {
>>> +            av_log(c, AV_LOG_ERROR, "Failed to read %d frome file\n", left);
>>> +            return ret;
>>> +        }
>>> +        c->len = 0;
>>> +    }
>>> +    return 0;
>>> +}
>>> +
>>> +static int low_overhead_get_packet(AVFormatContext *s, AVPacket *pkt)
>>> +{
>>> +    int64_t obu_size;
>>> +    int ret, type, has_size_flag;
>>> +    LowOverheadContext *c = s->priv_data;
>>> +    ret = low_overhead_prefetch(s);
>>> +    if (ret < 0)
>>> +        return ret;
>>> +    if (ret) {
>>> +        ret = read_obu(c->prefetched, c->len, &obu_size, &type,
>> &has_size_flag);
>>> +        if (ret < 0 && !has_size_flag) {
>>
>> Shouldn't this be (ret < 0 || !has_size_flag)?
> Yes you are right, will change.
> 
> 
>>
>>> +            av_log(c, AV_LOG_ERROR, "Failed to read obu\n");
>>> +            return ret;
>>> +        }
>>> +        ret = low_overhead_read_data(s, pkt, ret);
>>> +    }
>>> +    return ret;
>>> +}
>>> +
>>> +static int low_overhead_read_packet(AVFormatContext *s, AVPacket
>>> +*pkt) {
>>> +    LowOverheadContext *c = s->priv_data;
>>> +    int ret;
>>> +
>>> +    while (1) {
>>> +        ret = low_overhead_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 low_overhead_read_close(AVFormatContext *s) {
>>> +    LowOverheadContext *c = s->priv_data;
>>> +
>>> +    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(LowOverheadContext, x) static const
>>> +AVOption low_overhead_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", @@ -272,3 +427,23 @@
>>> AVInputFormat ff_av1_demuxer = {
>>>      .flags          = AVFMT_GENERIC_INDEX,
>>>      .priv_class     = &annexb_demuxer_class,
>>>  };
>>> +
>>> +static const AVClass low_overhead_demuxer_class = {
>>> +    .class_name = "AV1 low overhead OBU demuxer",
>>> +    .item_name  = av_default_item_name,
>>> +    .option     = low_overhead_options,
>>> +    .version    = LIBAVUTIL_VERSION_INT,
>>> +};
>>> +
>>> +AVInputFormat ff_av1_low_overhead_demuxer = {
>>> +    .name           = "av1",
>>
>> This name is already used by the AnnexB demuxer, so change it to "obu".
> Sure.
> 
>>
>>> +    .long_name      = NULL_IF_CONFIG_SMALL("AV1 low overhead OBU"),
>>> +    .priv_data_size = sizeof(LowOverheadContext),
>>> +    .read_probe     = low_overhead_probe,
>>> +    .read_header    = low_overhead_read_header,
>>> +    .read_packet    = low_overhead_read_packet,
>>> +    .read_close     = low_overhead_read_close,
>>> +    .extensions     = "obu",
>>> +    .flags          = AVFMT_GENERIC_INDEX,
>>> +    .priv_class     = &low_overhead_demuxer_class,
>>> +};
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email ffmpeg-devel-request@ffmpeg.org
>> with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
diff mbox series

Patch

diff --git a/libavformat/allformats.c b/libavformat/allformats.c
index fd9e46e233..8eead5619d 100644
--- a/libavformat/allformats.c
+++ b/libavformat/allformats.c
@@ -75,6 +75,7 @@  extern AVOutputFormat ff_asf_stream_muxer;
 extern AVInputFormat  ff_au_demuxer;
 extern AVOutputFormat ff_au_muxer;
 extern AVInputFormat  ff_av1_demuxer;
+extern AVInputFormat  ff_av1_low_overhead_demuxer;
 extern AVInputFormat  ff_avi_demuxer;
 extern AVOutputFormat ff_avi_muxer;
 extern AVInputFormat  ff_avisynth_demuxer;
diff --git a/libavformat/av1dec.c b/libavformat/av1dec.c
index ec66152e03..0c5d172a0f 100644
--- a/libavformat/av1dec.c
+++ b/libavformat/av1dec.c
@@ -28,6 +28,9 @@ 
 #include "avio_internal.h"
 #include "internal.h"
 
+//2 + max leb 128 size
+#define MAX_HEAD_SIZE 10
+
 typedef struct AnnexBContext {
     const AVClass *class;
     AVBSFContext *bsf;
@@ -139,9 +142,8 @@  static int annexb_probe(const AVProbeData *p)
     return 0;
 }
 
-static int annexb_read_header(AVFormatContext *s)
+static int read_header(AVFormatContext *s, AVRational framerate, AVBSFContext **bsf, void *c)
 {
-    AnnexBContext *c = s->priv_data;
     const AVBitStreamFilter *filter = av_bsf_get_by_name("av1_frame_merge");
     AVStream *st;
     int ret;
@@ -160,25 +162,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)
@@ -246,12 +255,158 @@  static int annexb_read_close(AVFormatContext *s)
     return 0;
 }
 
-#define OFFSET(x) offsetof(AnnexBContext, x)
+typedef struct LowOverheadContext {
+    const AVClass *class;
+    AVBSFContext *bsf;
+    AVRational framerate;
+    uint8_t prefetched[MAX_HEAD_SIZE];
+    //prefetched len
+    int len;
+} LowOverheadContext;
+
+static int low_overhead_probe(const AVProbeData *p)
+{
+    AVIOContext pb;
+    int64_t obu_size;
+    int ret, type, cnt = 0, has_size_flag;
+
+    ffio_init_context(&pb, p->buf, p->buf_size, 0,
+                      NULL, NULL, NULL, NULL);
+
+    // Check that the first OBU is a Temporal Delimiter.
+    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE), &obu_size, &type, &has_size_flag);
+    if (ret < 0 || type != AV1_OBU_TEMPORAL_DELIMITER || obu_size != 0 || !has_size_flag)
+        return 0;
+    cnt += ret;
+
+    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE), &obu_size, &type, &has_size_flag);
+    if (ret < 0 || type != AV1_OBU_SEQUENCE_HEADER || !has_size_flag)
+        return 0;
+    cnt += ret;
+
+    ret = read_obu(p->buf + cnt, FFMIN(p->buf_size - cnt, MAX_HEAD_SIZE), &obu_size, &type, &has_size_flag);
+    if (ret < 0 || (type != AV1_OBU_FRAME && type != AV1_OBU_FRAME_HEADER) || obu_size <= 0 || !has_size_flag)
+        return 0;
+    return AVPROBE_SCORE_EXTENSION + 1;
+}
+
+static int low_overhead_read_header(AVFormatContext *s)
+{
+    LowOverheadContext *c = s->priv_data;
+    return read_header(s, c->framerate, &c->bsf, c);
+}
+
+static int low_overhead_prefetch(AVFormatContext *s)
+{
+    LowOverheadContext *c = s->priv_data;
+    int ret;
+    int size = MAX_HEAD_SIZE - c->len;
+    if (size > 0 && !avio_feof(s->pb)) {
+        ret = avio_read(s->pb, &c->prefetched[c->len], size);
+        if (ret < 0)
+            return ret;
+        c->len += ret;
+    }
+    return c->len;
+}
+
+static int low_overhead_read_data(AVFormatContext *s, AVPacket *pkt, int size)
+{
+    int left;
+    LowOverheadContext *c = s->priv_data;
+    int ret = av_new_packet(pkt, size);
+    if (ret < 0) {
+        av_log(c, AV_LOG_ERROR, "Failed to allocate packet for obu\n");
+        return ret;
+    }
+    if (size <= c->len) {
+        memcpy(pkt->data, c->prefetched, size);
+
+        left = c->len - size;
+        memcpy(c->prefetched, c->prefetched + size, left);
+        c->len = left;
+    } else {
+        memcpy(pkt->data, c->prefetched, c->len);
+        left = size - c->len;
+        ret = avio_read(s->pb, pkt->data + c->len, left);
+        if (ret != left) {
+            av_log(c, AV_LOG_ERROR, "Failed to read %d frome file\n", left);
+            return ret;
+        }
+        c->len = 0;
+    }
+    return 0;
+}
+
+static int low_overhead_get_packet(AVFormatContext *s, AVPacket *pkt)
+{
+    int64_t obu_size;
+    int ret, type, has_size_flag;
+    LowOverheadContext *c = s->priv_data;
+    ret = low_overhead_prefetch(s);
+    if (ret < 0)
+        return ret;
+    if (ret) {
+        ret = read_obu(c->prefetched, c->len, &obu_size, &type, &has_size_flag);
+        if (ret < 0 && !has_size_flag) {
+            av_log(c, AV_LOG_ERROR, "Failed to read obu\n");
+            return ret;
+        }
+        ret = low_overhead_read_data(s, pkt, ret);
+    }
+    return ret;
+}
+
+static int low_overhead_read_packet(AVFormatContext *s, AVPacket *pkt)
+{
+    LowOverheadContext *c = s->priv_data;
+    int ret;
+
+    while (1) {
+        ret = low_overhead_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 low_overhead_read_close(AVFormatContext *s)
+{
+    LowOverheadContext *c = s->priv_data;
+
+    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(LowOverheadContext, x)
+static const AVOption low_overhead_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",
@@ -272,3 +427,23 @@  AVInputFormat ff_av1_demuxer = {
     .flags          = AVFMT_GENERIC_INDEX,
     .priv_class     = &annexb_demuxer_class,
 };
+
+static const AVClass low_overhead_demuxer_class = {
+    .class_name = "AV1 low overhead OBU demuxer",
+    .item_name  = av_default_item_name,
+    .option     = low_overhead_options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
+AVInputFormat ff_av1_low_overhead_demuxer = {
+    .name           = "av1",
+    .long_name      = NULL_IF_CONFIG_SMALL("AV1 low overhead OBU"),
+    .priv_data_size = sizeof(LowOverheadContext),
+    .read_probe     = low_overhead_probe,
+    .read_header    = low_overhead_read_header,
+    .read_packet    = low_overhead_read_packet,
+    .read_close     = low_overhead_read_close,
+    .extensions     = "obu",
+    .flags          = AVFMT_GENERIC_INDEX,
+    .priv_class     = &low_overhead_demuxer_class,
+};