[FFmpeg-devel] [PATCH] cbs_h2645: Implement replace-PS with a table rather than many functions

[Mark Thompson]

On 30/01/2021 11:01, Nuo Mi wrote:
> On Wed, Jan 27, 2021 at 9:52 PM Nuo Mi <nuomi2021 at gmail.com> wrote:
>> On Wed, Jan 27, 2021 at 7:06 AM Mark Thompson <sw at jkqxz.net> wrote:
>>>
>>> +
>>> +    err = ff_cbs_make_unit_refcounted(ctx, unit);
>>> +    if (err < 0)
>>> +        return err;
>>> +
>>> +    ref_array =
>>> +         (AVBufferRef**)((uint8_t*)ctx->priv_data +
>>> ps_type->ref_array_offset);
>>> +    ptr_array = (void**)((uint8_t*)ctx->priv_data +
>>> ps_type->ptr_array_offset);
>>> +    active    = (void**)((uint8_t*)ctx->priv_data +
>>> ps_type->active_offset);
>>> +
>>> +    if (ptr_array[id] == *active) {
>>> +        // The old active parameter set is being overwritten, so it can't
>>> +        // be active after this point.
>>> +        *active = NULL;
>>> +    }
>>> +    av_buffer_unref(&ref_array[id]);
>>> +
>>> +    ref_array[id] = av_buffer_ref(unit->content_ref);
>>> +    if (!ref_array[id])
>>> +        return AVERROR(ENOMEM);
>>>
>> This happend after ff_cbs_make_unit_refcounted, do we need urnef unit->content_ref
>> before return?

I don't think so?  The content_ref will be freed by the next call to fragment_reset/fragment_free, and we don't want to unset it here because that would force us to also clear unit->content as well (which doesn't matter for reading, but is a very strange side-effect if you are writing).

>>> +    ptr_array[id] = ref_array[id]->data;
>>> +
>>> +    return 0;
>>> +}
>>>
>>> > Hi Mark,
> Will this fix and merged?

Do you prefer it to what is presently there?

My intent was to suggest it and ask the question given the trouble with many versions of this in H.266, not to prescribe a particular answer.

- Mark