On 05/11/18 18:16, James Almer wrote: > 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_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()?
... which is also called from cbs_av1_read_frame_obu(), and may or may not be wanted in the redundant frame header case. It's all fixable with some more magic state in the other direction (maybe priv->make_frame_header_ref indicating that the caller should ref the current OBU buffer to remember the frame header), but that splits it between two places and isn't obviously any nicer. >> + } 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); >> + } >> } >> } >> >> ... - Mark _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel