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