[FFmpeg-devel] [PATCH 2/5 V2] avcodec: add an AV1 frame merge bitstream filter

[Andreas Rheinhardt]

James Almer:
> On 11/11/2019 8:36 PM, James Almer wrote:
>>>> +    for (i = 0; i < frag->nb_units; i++) {
>>>> +        if (i && frag->units[i].type == AV1_OBU_TEMPORAL_DELIMITER) {
>>>> +            av_log(bsf, AV_LOG_ERROR, "Temporal Delimiter in the middle of a packet.\n");
>>>> +            err = AVERROR_INVALIDDATA;
>>>> +            goto fail;
>>>> +        }
>>>> +        err = ff_cbs_insert_unit_content(ctx->cbc, tu, -1, frag->units[i].type,
>>>> +                                         frag->units[i].content, frag->units[i].content_ref);
>>> You make new references to content and then immediately thereafter
>>> unref the old ones. How about adding a function that moves a unit (or
>>> several units at once) from one fragment to another?
>> A new function that moves units sounds good. Shouldn't be hard to
>> implement either, i think.
> 
> So a unit move function would require a memcpy for the unit from one
> fragment to another, a memset to 0 on the source unit, then a call to
> ff_cbs_delete_unit() which memmoves the remaining units to fill the gap
> created by the removed one, if any. I don't know if it's that much
> better than this current implementation creating a new AVBufferRef per
> unit, then wiping the whole source fragment in one go.

On second thought I think that it's not worth the effort here. After
all, said function needs to be cbs-internal any way (it has to make
sure that there is enough space in the destination array and for this
it needs to examine and possibly modify the number of allocated units)
and therefore it should not be a simple "move a whole fragment to the
end of another fragment" (that's what I had in mind and this would be
pretty simple to implement and also pretty fast as no remaining units
needed to be moved).

But for the other place where you move units, nothing beats simply
swapping the roles of the two fragments.

- Andreas