[FFmpeg-devel] [PATCH 01/11] avcodec/vulkan_encode_h264: Fix memleak on error

[Mark Thompson]

On 06/05/2025 00:37, Andreas Rheinhardt wrote:
> Patches attached.
> 
> - Andreas
> 
> 
> [PATCH 05/11] avcodec/cbs,cbs_jpeg: Split ff_cbs_append_unit_data()

Suggest assert0 for all error cases in cbs generic code, since none of them are inner-loop performance sensitive.

Patch looks good.

> [PATCH 06/11] avcodec/cbs: Allow to avoid refcounting/data copying

Having force_refcounting set to one at init makes me think the sense of it should be other way around?  ("disable_refcounting"?)

Would it be sensible for the new checks for data_ref to assert that you are not in a refcounting case?

Also wondering whether we should have a new helper function something like:

int ff_cbs_make_unit_data_ref(SomethingContexty *c, AVBufferRef **ref, const CodedBitstreamUnit *unit)
{
    av_assert0(!*ref);
    if (unit->data_ref) {
        *ref = av_buffer_ref(unit->data_ref);
        if (!*ref)
            return AVERROR(ENOMEM);
    } else {
        av_assert0(c->disable_refcounting);
    }
    return 0;
}

to avoid the repeated code you add in the various codec files.

> [PATCH 07/11] avcodec/apv_parser: Don't use dummy AVBufferRef*
> [PATCH 08/11] Revert "avcodec/cbs: add an AVBufferRef input argument to ff_cbs_read()"

Yes, this ends up being a nicer answer.

> [PATCH 09/11] avcodec/av1_parser: Avoid copying data

It's not obvious that this does the right thing with the persistent reference to the sequence header object.  Can you explain further how that is working?

> [PATCH 10/11] avcodec/cbs_bsf: Avoid creating unnecessary references

Does this not add a significant new constraint on all bsfs using the cbs_bsf structure?  It's unclear to me whether works in all cases such as those where new units are added.

> [PATCH 11/11] avcodec/bsf/trace_headers: Avoid creating unnecessary references

Same thought as 9 in other codecs, but fine if 9 is.

Thanks,

- Mark