diff mbox

[FFmpeg-devel,3/5] avcodec/libaomenc: export Sequence Header and Metadata OBUs as extradata

Message ID 20180709182654.9996-4-jamrial@gmail.com
State Accepted
Commit a754af942a6ab0e98eac0da36fcf41ffca77c952
Headers show

Commit Message

James Almer July 9, 2018, 6:26 p.m. UTC 
aom_codec_get_global_headers() is not implemented as of libaom 1.0.0
for AV1, so we're forced to extract the relevant header OBUs from the
first packet and propagate them as packet side data for now.

Signed-off-by: James Almer <jamrial@gmail.com>
---
This is far from ideal. Whereas the mp4 muxer can handle extradata
propagated as packet side data without issues, the Matroska one can't
feasibly do it since it would require to reserve space for it, and we
don't know just how big the resulting extradata can be as it may have
an arbitrary amount of OBUs.

libaom should ideally implement aom_codec_get_global_headers() for
AV1 (Which is clearly inspired by similar functionality in libx264
and other encoders, and can be used before any kind of image data is
sent to the encoder), so lobby is welcome :p

 configure              |  1 +
 libavcodec/libaomenc.c | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

Comments

Niki Bowe July 9, 2018, 9:53 p.m. UTC | #1 
Hi James,
I passed along your wish for aom_codec_get_global_headers().
Tom created this bug for tracking:
https://bugs.chromium.org/p/aomedia/issues/detail?id=2012


On Mon, Jul 9, 2018 at 11:29 AM James Almer <jamrial@gmail.com> wrote:

> aom_codec_get_global_headers() is not implemented as of libaom 1.0.0
> for AV1, so we're forced to extract the relevant header OBUs from the
> first packet and propagate them as packet side data for now.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
> This is far from ideal. Whereas the mp4 muxer can handle extradata
> propagated as packet side data without issues, the Matroska one can't
> feasibly do it since it would require to reserve space for it, and we
> don't know just how big the resulting extradata can be as it may have
> an arbitrary amount of OBUs.
>
> libaom should ideally implement aom_codec_get_global_headers() for
> AV1 (Which is clearly inspired by similar functionality in libx264
> and other encoders, and can be used before any kind of image data is
> sent to the encoder), so lobby is welcome :p
>
>  configure              |  1 +
>  libavcodec/libaomenc.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 42 insertions(+)
>
> diff --git a/configure b/configure
> index 1066df6621..a76dd06736 100755
> --- a/configure
> +++ b/configure
> @@ -3046,6 +3046,7 @@ hevc_videotoolbox_encoder_deps="pthreads"
>  hevc_videotoolbox_encoder_select="videotoolbox_encoder"
>  libaom_av1_decoder_deps="libaom"
>  libaom_av1_encoder_deps="libaom"
> +libaom_av1_encoder_select="extract_extradata_bsf"
>  libcelt_decoder_deps="libcelt"
>  libcodec2_decoder_deps="libcodec2"
>  libcodec2_encoder_deps="libcodec2"
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index 41b05dc1c0..0b75dc139c 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -55,6 +55,7 @@ struct FrameListData {
>
>  typedef struct AOMEncoderContext {
>      AVClass *class;
> +    AVBSFContext *bsf;
>      struct aom_codec_ctx encoder;
>      struct aom_image rawimg;
>      struct aom_fixed_buf twopass_stats;
> @@ -202,6 +203,7 @@ static av_cold int aom_free(AVCodecContext *avctx)
>      av_freep(&ctx->twopass_stats.buf);
>      av_freep(&avctx->stats_out);
>      free_frame_list(ctx->coded_frame_list);
> +    av_bsf_free(&ctx->bsf);
>      return 0;
>  }
>
> @@ -463,6 +465,28 @@ static av_cold int aom_init(AVCodecContext *avctx,
>      if (!cpb_props)
>          return AVERROR(ENOMEM);
>
> +    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> +        const AVBitStreamFilter *filter =
> av_bsf_get_by_name("extract_extradata");
> +        int ret;
> +
> +        if (!filter) {
> +            av_log(avctx, AV_LOG_ERROR, "extract_extradata bitstream
> filter "
> +                   "not found. This is a bug, please report it.\n");
> +            return AVERROR_BUG;
> +        }
> +        ret = av_bsf_alloc(filter, &ctx->bsf);
> +        if (ret < 0)
> +            return ret;
> +
> +        ret = avcodec_parameters_from_context(ctx->bsf->par_in, avctx);
> +        if (ret < 0)
> +           return ret;
> +
> +        ret = av_bsf_init(ctx->bsf);
> +        if (ret < 0)
> +           return ret;
> +    }
> +
>      if (enccfg.rc_end_usage == AOM_CBR ||
>          enccfg.g_pass != AOM_RC_ONE_PASS) {
>          cpb_props->max_bitrate = avctx->rc_max_rate;
> @@ -494,6 +518,7 @@ static inline void cx_pktcpy(struct FrameListData *dst,
>  static int storeframe(AVCodecContext *avctx, struct FrameListData
> *cx_frame,
>                        AVPacket *pkt)
>  {
> +    AOMContext *ctx = avctx->priv_data;
>      int ret = ff_alloc_packet2(avctx, pkt, cx_frame->sz, 0);
>      if (ret < 0) {
>          av_log(avctx, AV_LOG_ERROR,
> @@ -505,6 +530,22 @@ static int storeframe(AVCodecContext *avctx, struct
> FrameListData *cx_frame,
>
>      if (!!(cx_frame->flags & AOM_FRAME_IS_KEY))
>          pkt->flags |= AV_PKT_FLAG_KEY;
> +
> +    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> +        ret = av_bsf_send_packet(ctx->bsf, pkt);
> +        if (ret < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> +                   "failed to send input packet\n");
> +            return ret;
> +        }
> +        ret = av_bsf_receive_packet(ctx->bsf, pkt);
> +
> +        if (ret < 0) {
> +            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
> +                   "failed to receive output packet\n");
> +            return ret;
> +        }
> +    }
>      return pkt->size;
>  }
>
> --
> 2.18.0
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
James Almer July 9, 2018, 10:52 p.m. UTC | #2 
On 7/9/2018 6:53 PM, Niki Bowe wrote:
> Hi James,
> I passed along your wish for aom_codec_get_global_headers().
> Tom created this bug for tracking:
> https://bugs.chromium.org/p/aomedia/issues/detail?id=2012

Thanks a lot! It would simplify the libaom wrapper a lot and actually
allow us to write the extradata in Matroska directly during transcoding
without requiring some big intrusive changes to that muxer.

To be more specific, aom_codec_get_global_headers() should be callable
and return a sequence header OBU (and any relevant Metadata OBU) after
calling aom_codec_enc_init() but before the first aom_codec_encode()
call. This is what x264_encoder_headers() in libx264 and
ISVCEncoder::EncodeParameterSets in libopenh264 let you do, for example.
Requiring first to pass image data with aom_codec_encode() to be able to
generate the Sequence Header would make virtually no difference compared
to what I'm implementing in this patch.

I see for that matter that aom_codec_control() can be called after
aom_codec_enc_init() and it evidently can have an effect in the eventual
contents of the Sequence Header. Knowing this, and assuming it's not the
case already, perhaps certain aome_enc_control_id values (like for
example color config related ones) should be forbidden to be set after
aom_codec_get_global_headers() is called. Otherwise the headers the
function returns could become invalid if aom_codec_control() was
successfully called afterwards.

> 
> 
> On Mon, Jul 9, 2018 at 11:29 AM James Almer <jamrial@gmail.com> wrote:
> 
>> aom_codec_get_global_headers() is not implemented as of libaom 1.0.0
>> for AV1, so we're forced to extract the relevant header OBUs from the
>> first packet and propagate them as packet side data for now.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>> This is far from ideal. Whereas the mp4 muxer can handle extradata
>> propagated as packet side data without issues, the Matroska one can't
>> feasibly do it since it would require to reserve space for it, and we
>> don't know just how big the resulting extradata can be as it may have
>> an arbitrary amount of OBUs.
>>
>> libaom should ideally implement aom_codec_get_global_headers() for
>> AV1 (Which is clearly inspired by similar functionality in libx264
>> and other encoders, and can be used before any kind of image data is
>> sent to the encoder), so lobby is welcome :p
>>
>>  configure              |  1 +
>>  libavcodec/libaomenc.c | 41 +++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 42 insertions(+)
>>
>> diff --git a/configure b/configure
>> index 1066df6621..a76dd06736 100755
>> --- a/configure
>> +++ b/configure
>> @@ -3046,6 +3046,7 @@ hevc_videotoolbox_encoder_deps="pthreads"
>>  hevc_videotoolbox_encoder_select="videotoolbox_encoder"
>>  libaom_av1_decoder_deps="libaom"
>>  libaom_av1_encoder_deps="libaom"
>> +libaom_av1_encoder_select="extract_extradata_bsf"
>>  libcelt_decoder_deps="libcelt"
>>  libcodec2_decoder_deps="libcodec2"
>>  libcodec2_encoder_deps="libcodec2"
>> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
>> index 41b05dc1c0..0b75dc139c 100644
>> --- a/libavcodec/libaomenc.c
>> +++ b/libavcodec/libaomenc.c
>> @@ -55,6 +55,7 @@ struct FrameListData {
>>
>>  typedef struct AOMEncoderContext {
>>      AVClass *class;
>> +    AVBSFContext *bsf;
>>      struct aom_codec_ctx encoder;
>>      struct aom_image rawimg;
>>      struct aom_fixed_buf twopass_stats;
>> @@ -202,6 +203,7 @@ static av_cold int aom_free(AVCodecContext *avctx)
>>      av_freep(&ctx->twopass_stats.buf);
>>      av_freep(&avctx->stats_out);
>>      free_frame_list(ctx->coded_frame_list);
>> +    av_bsf_free(&ctx->bsf);
>>      return 0;
>>  }
>>
>> @@ -463,6 +465,28 @@ static av_cold int aom_init(AVCodecContext *avctx,
>>      if (!cpb_props)
>>          return AVERROR(ENOMEM);
>>
>> +    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
>> +        const AVBitStreamFilter *filter =
>> av_bsf_get_by_name("extract_extradata");
>> +        int ret;
>> +
>> +        if (!filter) {
>> +            av_log(avctx, AV_LOG_ERROR, "extract_extradata bitstream
>> filter "
>> +                   "not found. This is a bug, please report it.\n");
>> +            return AVERROR_BUG;
>> +        }
>> +        ret = av_bsf_alloc(filter, &ctx->bsf);
>> +        if (ret < 0)
>> +            return ret;
>> +
>> +        ret = avcodec_parameters_from_context(ctx->bsf->par_in, avctx);
>> +        if (ret < 0)
>> +           return ret;
>> +
>> +        ret = av_bsf_init(ctx->bsf);
>> +        if (ret < 0)
>> +           return ret;
>> +    }
>> +
>>      if (enccfg.rc_end_usage == AOM_CBR ||
>>          enccfg.g_pass != AOM_RC_ONE_PASS) {
>>          cpb_props->max_bitrate = avctx->rc_max_rate;
>> @@ -494,6 +518,7 @@ static inline void cx_pktcpy(struct FrameListData *dst,
>>  static int storeframe(AVCodecContext *avctx, struct FrameListData
>> *cx_frame,
>>                        AVPacket *pkt)
>>  {
>> +    AOMContext *ctx = avctx->priv_data;
>>      int ret = ff_alloc_packet2(avctx, pkt, cx_frame->sz, 0);
>>      if (ret < 0) {
>>          av_log(avctx, AV_LOG_ERROR,
>> @@ -505,6 +530,22 @@ static int storeframe(AVCodecContext *avctx, struct
>> FrameListData *cx_frame,
>>
>>      if (!!(cx_frame->flags & AOM_FRAME_IS_KEY))
>>          pkt->flags |= AV_PKT_FLAG_KEY;
>> +
>> +    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
>> +        ret = av_bsf_send_packet(ctx->bsf, pkt);
>> +        if (ret < 0) {
>> +            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
>> +                   "failed to send input packet\n");
>> +            return ret;
>> +        }
>> +        ret = av_bsf_receive_packet(ctx->bsf, pkt);
>> +
>> +        if (ret < 0) {
>> +            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
>> +                   "failed to receive output packet\n");
>> +            return ret;
>> +        }
>> +    }
>>      return pkt->size;
>>  }
>>
>> --
>> 2.18.0
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
> 
>
diff mbox

Patch

diff --git a/configure b/configure
index 1066df6621..a76dd06736 100755
--- a/configure
+++ b/configure
@@ -3046,6 +3046,7 @@  hevc_videotoolbox_encoder_deps="pthreads"
 hevc_videotoolbox_encoder_select="videotoolbox_encoder"
 libaom_av1_decoder_deps="libaom"
 libaom_av1_encoder_deps="libaom"
+libaom_av1_encoder_select="extract_extradata_bsf"
 libcelt_decoder_deps="libcelt"
 libcodec2_decoder_deps="libcodec2"
 libcodec2_encoder_deps="libcodec2"
diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 41b05dc1c0..0b75dc139c 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -55,6 +55,7 @@  struct FrameListData {
 
 typedef struct AOMEncoderContext {
     AVClass *class;
+    AVBSFContext *bsf;
     struct aom_codec_ctx encoder;
     struct aom_image rawimg;
     struct aom_fixed_buf twopass_stats;
@@ -202,6 +203,7 @@  static av_cold int aom_free(AVCodecContext *avctx)
     av_freep(&ctx->twopass_stats.buf);
     av_freep(&avctx->stats_out);
     free_frame_list(ctx->coded_frame_list);
+    av_bsf_free(&ctx->bsf);
     return 0;
 }
 
@@ -463,6 +465,28 @@  static av_cold int aom_init(AVCodecContext *avctx,
     if (!cpb_props)
         return AVERROR(ENOMEM);
 
+    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
+        const AVBitStreamFilter *filter = av_bsf_get_by_name("extract_extradata");
+        int ret;
+
+        if (!filter) {
+            av_log(avctx, AV_LOG_ERROR, "extract_extradata bitstream filter "
+                   "not found. This is a bug, please report it.\n");
+            return AVERROR_BUG;
+        }
+        ret = av_bsf_alloc(filter, &ctx->bsf);
+        if (ret < 0)
+            return ret;
+
+        ret = avcodec_parameters_from_context(ctx->bsf->par_in, avctx);
+        if (ret < 0)
+           return ret;
+
+        ret = av_bsf_init(ctx->bsf);
+        if (ret < 0)
+           return ret;
+    }
+
     if (enccfg.rc_end_usage == AOM_CBR ||
         enccfg.g_pass != AOM_RC_ONE_PASS) {
         cpb_props->max_bitrate = avctx->rc_max_rate;
@@ -494,6 +518,7 @@  static inline void cx_pktcpy(struct FrameListData *dst,
 static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
                       AVPacket *pkt)
 {
+    AOMContext *ctx = avctx->priv_data;
     int ret = ff_alloc_packet2(avctx, pkt, cx_frame->sz, 0);
     if (ret < 0) {
         av_log(avctx, AV_LOG_ERROR,
@@ -505,6 +530,22 @@  static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
 
     if (!!(cx_frame->flags & AOM_FRAME_IS_KEY))
         pkt->flags |= AV_PKT_FLAG_KEY;
+
+    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
+        ret = av_bsf_send_packet(ctx->bsf, pkt);
+        if (ret < 0) {
+            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
+                   "failed to send input packet\n");
+            return ret;
+        }
+        ret = av_bsf_receive_packet(ctx->bsf, pkt);
+
+        if (ret < 0) {
+            av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
+                   "failed to receive output packet\n");
+            return ret;
+        }
+    }
     return pkt->size;
 }