[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 05:22:55 EET 2018


On 3/17/2018 11:13 PM, James Almer wrote:
> 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
> functions.
> 
> 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.

Actually no, aac_adtstoasc doesn't change the packet data, just the size
and data pointer like many others, so nevermind.

I'm dropping this patch, just in case. Sorry for the noise.

> 
> 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