diff mbox series

[FFmpeg-devel] avcodec/libaomenc: Extract extradata directly if available

Message ID 20200912143936.200522-1-andriy.gelman@gmail.com
State New
Headers show
Series [FFmpeg-devel] avcodec/libaomenc: Extract extradata directly if available
Related show

Checks

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

Commit Message

Andriy Gelman Sept. 12, 2020, 2:39 p.m. UTC
From: Andriy Gelman <andriy.gelman@gmail.com>

Currently a bsf is inserted to extract OBU headers which are propagated
as packet side data. The side data is used in matroska to insert out of
band extradata.

Support to fetch extradata directly from av1 was added in commit
f8d6a1653476662a7b00a1564afe37516adeba1a in libaom. So there is no need
to insert a bsf if extradata can be fetched.

Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
---
 libavcodec/libaomenc.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

James Almer Sept. 12, 2020, 3:48 p.m. UTC | #1
On 9/12/2020 11:39 AM, Andriy Gelman wrote:
> From: Andriy Gelman <andriy.gelman@gmail.com>
> 
> Currently a bsf is inserted to extract OBU headers which are propagated
> as packet side data. The side data is used in matroska to insert out of
> band extradata.
> 
> Support to fetch extradata directly from av1 was added in commit
> f8d6a1653476662a7b00a1564afe37516adeba1a in libaom. So there is no need
> to insert a bsf if extradata can be fetched.

No, see https://bugs.chromium.org/p/aomedia/issues/detail?id=2208

aom_codec_get_global_headers() has a bug that hasn't been fixed. That's
why i haven't implemented it yet in our wrapper.

> 
> Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> ---
>  libavcodec/libaomenc.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
> index 2b0581b15a..27ee630382 100644
> --- a/libavcodec/libaomenc.c
> +++ b/libavcodec/libaomenc.c
> @@ -879,6 +879,18 @@ static av_cold int aom_init(AVCodecContext *avctx,
>          return AVERROR(ENOMEM);
>  
>      if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> +        aom_fixed_buf_t *aom_buf;
> +        aom_buf = aom_codec_get_global_headers(&ctx->encoder);
> +        if (aom_buf) {
> +            avctx->extradata = av_mallocz(aom_buf->sz + AV_INPUT_BUFFER_PADDING_SIZE);
> +            memcpy(avctx->extradata, aom_buf->buf, aom_buf->sz);
> +            avctx->extradata_size = aom_buf->sz;
> +            av_free(aom_buf->buf);
> +            av_freep(&aom_buf);
> +        }
> +    }
> +
> +    if (!avctx->extradata && avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
>          const AVBitStreamFilter *filter = av_bsf_get_by_name("extract_extradata");
>          int ret;
>  
> @@ -976,7 +988,7 @@ static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
>  #endif
>      }
>  
> -    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
> +    if (ctx->bsf) {
>          ret = av_bsf_send_packet(ctx->bsf, pkt);
>          if (ret < 0) {
>              av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "
>
Derek Buitenhuis Sept. 12, 2020, 4:32 p.m. UTC | #2
On 12/09/2020 16:48, James Almer wrote:
> No, see https://bugs.chromium.org/p/aomedia/issues/detail?id=2208
> 
> aom_codec_get_global_headers() has a bug that hasn't been fixed. That's
> why i haven't implemented it yet in our wrapper.

Even if fixed, shouldn't be still use the BSF anyway, since it could
also add in other useful config OBUs like HDR metadata OBUs?

- Derek
James Almer Sept. 12, 2020, 5 p.m. UTC | #3
On 9/12/2020 1:32 PM, Derek Buitenhuis wrote:
> On 12/09/2020 16:48, James Almer wrote:
>> No, see https://bugs.chromium.org/p/aomedia/issues/detail?id=2208
>>
>> aom_codec_get_global_headers() has a bug that hasn't been fixed. That's
>> why i haven't implemented it yet in our wrapper.
> 
> Even if fixed, shouldn't be still use the BSF anyway, since it could
> also add in other useful config OBUs like HDR metadata OBUs?

I'd expect aom_codec_get_global_headers() to return all OBUs that apply
to the entire coded stream, hence the name, so that should include
Metadata OBUs like HDR (but not frame specific ones like ITU-T 35).

But if it doesn't then yeah, we wouldn't be able to remove the extract
extradata bsf here.

> 
> - Derek
> _______________________________________________
> 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".
>
Andriy Gelman Sept. 12, 2020, 7:49 p.m. UTC | #4
On Sat, 12. Sep 14:00, James Almer wrote:
> On 9/12/2020 1:32 PM, Derek Buitenhuis wrote:
> > On 12/09/2020 16:48, James Almer wrote:
> >> No, see https://bugs.chromium.org/p/aomedia/issues/detail?id=2208
> >>
> >> aom_codec_get_global_headers() has a bug that hasn't been fixed. That's
> >> why i haven't implemented it yet in our wrapper.
> > 
> > Even if fixed, shouldn't be still use the BSF anyway, since it could
> > also add in other useful config OBUs like HDR metadata OBUs?
> 
> I'd expect aom_codec_get_global_headers() to return all OBUs that apply
> to the entire coded stream, hence the name, so that should include
> Metadata OBUs like HDR (but not frame specific ones like ITU-T 35).
> 
> But if it doesn't then yeah, we wouldn't be able to remove the extract
> extradata bsf here.
> 

They only write the sequence header OBU at the moment.
diff mbox series

Patch

diff --git a/libavcodec/libaomenc.c b/libavcodec/libaomenc.c
index 2b0581b15a..27ee630382 100644
--- a/libavcodec/libaomenc.c
+++ b/libavcodec/libaomenc.c
@@ -879,6 +879,18 @@  static av_cold int aom_init(AVCodecContext *avctx,
         return AVERROR(ENOMEM);
 
     if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
+        aom_fixed_buf_t *aom_buf;
+        aom_buf = aom_codec_get_global_headers(&ctx->encoder);
+        if (aom_buf) {
+            avctx->extradata = av_mallocz(aom_buf->sz + AV_INPUT_BUFFER_PADDING_SIZE);
+            memcpy(avctx->extradata, aom_buf->buf, aom_buf->sz);
+            avctx->extradata_size = aom_buf->sz;
+            av_free(aom_buf->buf);
+            av_freep(&aom_buf);
+        }
+    }
+
+    if (!avctx->extradata && avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
         const AVBitStreamFilter *filter = av_bsf_get_by_name("extract_extradata");
         int ret;
 
@@ -976,7 +988,7 @@  static int storeframe(AVCodecContext *avctx, struct FrameListData *cx_frame,
 #endif
     }
 
-    if (avctx->flags & AV_CODEC_FLAG_GLOBAL_HEADER) {
+    if (ctx->bsf) {
         ret = av_bsf_send_packet(ctx->bsf, pkt);
         if (ret < 0) {
             av_log(avctx, AV_LOG_ERROR, "extract_extradata filter "