[FFmpeg-devel] [PATCH v2] avcodec/bsf: restructure the internal implementation of the bistream filter API

[James Almer]

On 4/19/2020 4:59 PM, Andreas Rheinhardt wrote:
>>  int av_bsf_receive_packet(AVBSFContext *ctx, AVPacket *pkt)
>>  {
>> -    return ctx->filter->filter(ctx, pkt);
>> +    AVBSFInternal *bsfi = ctx->internal;
>> +    int ret;
>> +
>> +    if (!bsfi->out_pkt->data && !bsfi->out_pkt->side_data_elems) {
>> +        ret = ctx->filter->filter(ctx, bsfi->out_pkt);
> The way you are doing this leads to an unnecessary av_packet_move_ref()
> and an unnecessary check (you can just "return ctx->filter->filter(ctx,
> pkt);"). Given that the doc states that the supplied pkt has to be clean
> and given that the filters unref it on error if they have modified it,
> the packet will be clean on error. If you worry that some padding bytes
> might have been modified, then change the doc to "On failure, pkt will
> be clean/blank on return" instead of adding an av_packet_move_ref().

"Should" is not "must". And even if it was, the user could still
mistakenly pass an uninitialized packet that an underlying bsf could
wrongly try to utilize (Calling av_grow_packet() instead of
av_new_packet(), calling av_packet_add_side_data(), or any kind of usage
that could behave unpredictably when there's existing data). So to
ensure bsfs will get a clean packet, the generic code should take care
of it, and this is the easiest way without outright returning an error
if an non blank packet is provided.

A call to av_pkt_move_ref() are almost free, does not care what's in the
dst packet, and is worth it for the extra assurance that bsfs will only
deal with clean packets from now on.