[FFmpeg-devel] [PATCH v2 2/2] cbs_av1: Support redundant frame headers
James Almer
jamrial at gmail.com
Mon Nov 5 20:16:29 EET 2018
On 11/5/2018 12:25 PM, Mark Thompson wrote:
> ---
> On 05/11/18 00:55, James Almer wrote:
>> On 11/4/2018 9:10 PM, Mark Thompson wrote:
>>> ...
>>> + xf(1, frame_header_copy[k], b, b, b, 1, k);
>> This is making a lot of noise in the trace output for no real gain,
>> since it prints every single bit as its own line. Can you silence it?
> I think it's sensible to keep some trace output here to reflect what's actually happening, so I've made it go by bytes rather than bits to be less spammy.
>
>>> + priv->frame_header_size = fh_bits;
>>> + priv->frame_header =
>>> + av_mallocz(fh_bytes + AV_INPUT_BUFFER_PADDING_SIZE);
>>> + if (!priv->frame_header)
>>> + return AVERROR(ENOMEM);
>>> + memcpy(priv->frame_header, fh_start, fh_bytes);
>> No way to use AVBufferRef for this?
> Refcounting added only for reading. It's a bit nasty because it passes the bufferref down into the template code which shouldn't really have it, but I don't see any better way to make that work.
>
>>> ...
> Also fixed the memory leak.
>
> Thanks,
>
> - Mark
>
>
> libavcodec/cbs_av1.c | 16 ++++--
> libavcodec/cbs_av1.h | 5 +-
> libavcodec/cbs_av1_syntax_template.c | 82 +++++++++++++++++++++++++---
> 3 files changed, 91 insertions(+), 12 deletions(-)
>
> diff --git a/libavcodec/cbs_av1.c b/libavcodec/cbs_av1.c
> index 1c49d90f51..ff32a6fca5 100644
> --- a/libavcodec/cbs_av1.c
> +++ b/libavcodec/cbs_av1.c
> @@ -996,7 +996,10 @@ static int cbs_av1_read_unit(CodedBitstreamContext *ctx,
> case AV1_OBU_REDUNDANT_FRAME_HEADER:
> {
> err = cbs_av1_read_frame_header_obu(ctx, &gbc,
> - &obu->obu.frame_header);
> + &obu->obu.frame_header,
> + obu->header.obu_type ==
> + AV1_OBU_REDUNDANT_FRAME_HEADER,
> + unit->data_ref);
> if (err < 0)
> return err;
> }
> @@ -1016,7 +1019,8 @@ static int cbs_av1_read_unit(CodedBitstreamContext *ctx,
> break;
> case AV1_OBU_FRAME:
> {
> - err = cbs_av1_read_frame_obu(ctx, &gbc, &obu->obu.frame);
> + err = cbs_av1_read_frame_obu(ctx, &gbc, &obu->obu.frame,
> + unit->data_ref);
> if (err < 0)
> return err;
>
> @@ -1124,7 +1128,10 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> case AV1_OBU_REDUNDANT_FRAME_HEADER:
> {
> err = cbs_av1_write_frame_header_obu(ctx, pbc,
> - &obu->obu.frame_header);
> + &obu->obu.frame_header,
> + obu->header.obu_type ==
> + AV1_OBU_REDUNDANT_FRAME_HEADER,
> + NULL);
> if (err < 0)
> return err;
> }
> @@ -1141,7 +1148,7 @@ static int cbs_av1_write_obu(CodedBitstreamContext *ctx,
> break;
> case AV1_OBU_FRAME:
> {
> - err = cbs_av1_write_frame_obu(ctx, pbc, &obu->obu.frame);
> + err = cbs_av1_write_frame_obu(ctx, pbc, &obu->obu.frame, NULL);
> if (err < 0)
> return err;
>
> @@ -1302,6 +1309,7 @@ static void cbs_av1_close(CodedBitstreamContext *ctx)
> CodedBitstreamAV1Context *priv = ctx->priv_data;
>
> av_buffer_unref(&priv->sequence_header_ref);
> + av_buffer_unref(&priv->frame_header_ref);
>
> av_freep(&priv->write_buffer);
> }
> diff --git a/libavcodec/cbs_av1.h b/libavcodec/cbs_av1.h
> index 614a0bf108..f662265f75 100644
> --- a/libavcodec/cbs_av1.h
> +++ b/libavcodec/cbs_av1.h
> @@ -399,7 +399,10 @@ typedef struct CodedBitstreamAV1Context {
> AV1RawSequenceHeader *sequence_header;
> AVBufferRef *sequence_header_ref;
>
> - int seen_frame_header;
> + int seen_frame_header;
> + AVBufferRef *frame_header_ref;
> + uint8_t *frame_header;
> + size_t frame_header_size;
>
> int temporal_id;
> int spatial_id;
> diff --git a/libavcodec/cbs_av1_syntax_template.c b/libavcodec/cbs_av1_syntax_template.c
> index e146bbf8bb..0da79b615d 100644
> --- a/libavcodec/cbs_av1_syntax_template.c
> +++ b/libavcodec/cbs_av1_syntax_template.c
> @@ -1463,24 +1463,90 @@ static int FUNC(uncompressed_header)(CodedBitstreamContext *ctx, RWContext *rw,
> }
>
> static int FUNC(frame_header_obu)(CodedBitstreamContext *ctx, RWContext *rw,
> - AV1RawFrameHeader *current)
> + AV1RawFrameHeader *current, int redundant,
> + AVBufferRef *rw_buffer_ref)
> {
> CodedBitstreamAV1Context *priv = ctx->priv_data;
> - int err;
> -
> - HEADER("Frame Header");
> + int start_pos, fh_bits, fh_bytes, err;
> + uint8_t *fh_start;
>
> if (priv->seen_frame_header) {
> - // Nothing to do.
> + if (!redundant) {
> + av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid repeated "
> + "frame header OBU.\n");
> + return AVERROR_INVALIDDATA;
> + } else {
> + GetBitContext fh;
> + size_t i, b;
> + uint32_t val;
> +
> + HEADER("Redundant Frame Header");
> +
> + av_assert0(priv->frame_header_ref && priv->frame_header);
> +
> + init_get_bits(&fh, priv->frame_header,
> + priv->frame_header_size);
> + for (i = 0; i < priv->frame_header_size; i += 8) {
> + b = FFMIN(priv->frame_header_size - i, 8);
> + val = get_bits(&fh, b);
> + xf(b, frame_header_copy[i],
> + val, val, val, 1, i / 8);
> + }
> + }
> } else {
> + if (redundant)
> + HEADER("Redundant Frame Header (used as Frame Header)");
> + else
> + HEADER("Frame Header");
> +
> priv->seen_frame_header = 1;
>
> +#ifdef READ
> + start_pos = get_bits_count(rw);
> +#else
> + start_pos = put_bits_count(rw);
> +#endif
> +
> CHECK(FUNC(uncompressed_header)(ctx, rw, current));
>
> if (current->show_existing_frame) {
> priv->seen_frame_header = 0;
> } else {
> priv->seen_frame_header = 1;
> +
> + av_buffer_unref(&priv->frame_header_ref);
> +
> +#ifdef READ
> + fh_bits = get_bits_count(rw) - start_pos;
> + fh_start = (uint8_t*)rw->buffer + start_pos / 8;
> +#else
> + // Need to flush the bitwriter so that we can copy its output,
> + // but use a copy so we don't affect the caller's structure.
> + {
> + PutBitContext tmp = *rw;
> + flush_put_bits(&tmp);
> + }
> +
> + fh_bits = put_bits_count(rw) - start_pos;
> + fh_start = rw->buf + start_pos / 8;
> +#endif
> + fh_bytes = (fh_bits + 7) / 8;
> +
> + priv->frame_header_size = fh_bits;
> +
> + if (rw_buffer_ref) {
> + priv->frame_header_ref = av_buffer_ref(rw_buffer_ref);
> + if (!priv->frame_header_ref)
> + return AVERROR(ENOMEM);
> + priv->frame_header = fh_start;
Is this the reason you can't create the ref outside the template code?
If it's only to skip the OBU header bits, can't that be done right after
the call to cbs_av1_read_frame_header_obu()?
> + } else {
> + priv->frame_header_ref =
> + av_buffer_alloc(fh_bytes + AV_INPUT_BUFFER_PADDING_SIZE);
> + if (!priv->frame_header_ref)
> + return AVERROR(ENOMEM);
> + priv->frame_header = priv->frame_header_ref->data;
> + memcpy(priv->frame_header, fh_start, fh_bytes);
> + }
> }
> }
>
> @@ -1524,11 +1590,13 @@ static int FUNC(tile_group_obu)(CodedBitstreamContext *ctx, RWContext *rw,
> }
>
> static int FUNC(frame_obu)(CodedBitstreamContext *ctx, RWContext *rw,
> - AV1RawFrame *current)
> + AV1RawFrame *current,
> + AVBufferRef *rw_buffer_ref)
> {
> int err;
>
> - CHECK(FUNC(frame_header_obu)(ctx, rw, ¤t->header));
> + CHECK(FUNC(frame_header_obu)(ctx, rw, ¤t->header,
> + 0, rw_buffer_ref));
>
> CHECK(FUNC(byte_alignment)(ctx, rw));
>
> -- 2.19.1 _______________________________________________ ffmpeg-devel
> mailing list ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
More information about the ffmpeg-devel
mailing list