[FFmpeg-devel] [PATCH 2/2] avcodec/noise_bsf: move the reference in the bsf internal buffer

James Almer jamrial at gmail.com
Sun Mar 18 04:13:01 EET 2018

On 3/17/2018 10:54 PM, Michael Niedermayer wrote:
> On Sat, Mar 17, 2018 at 10:39:25PM -0300, James Almer wrote:
>> On 3/17/2018 10:28 PM, Michael Niedermayer wrote:
>>> On Fri, Mar 16, 2018 at 08:16:01PM -0300, James Almer wrote:
>>>> There's no need to allocate a new packet for it.
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>>  libavcodec/noise_bsf.c | 30 ++++++++----------------------
>>>>  1 file changed, 8 insertions(+), 22 deletions(-)
>>> should be ok assumung the the packet from ff_bsf_get_packet_ref can be
>>> modified, this is not documented in bsf.h
>> Yes, ff_bsf_get_packet_ref() moves the reference from the internal bsf
>> packet as stored by av_bsf_send_packet() to the already allocated packet
>> passed by the caller, whereas ff_bsf_get_packet() hands a newly
>> allocated packet with the reference to the caller.
> what i meant is that the docs dont say that the caller can
> modify pkt->data without using some *make_writable() function

Turns outs the packet passed to the caller is the same one with both

ff_bsf_get_packet() does

    tmp_pkt = av_packet_alloc();
    if (!tmp_pkt)
        return AVERROR(ENOMEM);

    *pkt = ctx->internal->buffer_pkt;
    ctx->internal->buffer_pkt = tmp_pkt;

Meaning it's not handing over a newly allocated packet but the buffered
one instead.

ff_bsf_get_packet_ref() in turn does

    av_packet_move_ref(pkt, ctx->internal->buffer_pkt);

In both cases, ctx->internal->buffer_pkt is evidently expected to be
writable by the time a bsf requests it, at least judging by how it's
handled. See aac_adtstoasc, for example.

I guess a make_writable() call in av_bsf_send_packet() would be a good
idea to make sure that's effectively the case?
Neither the doxy for av_bsf_send_packet() or ff_bsf_get_packet* mention
anything about the writable status of this stored packet.

> also while ff_bsf_get_packet() documents freeing requirements,
> ff_bsf_get_packet_ref leaves this ambigous, from must over can
> to a must not.
> that is the docs should be made clearer assuming it is all correct
> [...]
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

More information about the ffmpeg-devel mailing list