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 <jamr...@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. 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@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel