[FFmpeg-devel] [PATCH 2/2] avcodec/noise_bsf: move the reference in the bsf internal buffer
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
> 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
More information about the ffmpeg-devel