| Message ID | 20200912143936.200522-1-andriy.gelman@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avcodec/libaomenc: Extract extradata directly if available | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
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 " >
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
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". >
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 --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 "