On 6/17/2019 12:42 AM, Andreas Rheinhardt wrote: > Up until now, ff_cbs_write_packet always initialized the packet > structure it received without documenting this behaviour; furthermore, > the packet's buffer would (on success) be overwritten with the new > buffer without unreferencing the old. This meant that the input packet > had to be either clean (otherwise there would be memleaks) in which case > the initialization is redundant or uninitialized. ff_cbs_write_packet > was never used with uninitialized packets, so the initialization was > redundant. Worse yet, it forced callers to use more than one packet and > made it difficult to add side-data to a packet designated for output, > because said side-data could only be attached after the call to > ff_cbs_write_packet. > > This has been changed. It is now allowed to use a non-blank packet. > The currently existing buffer will be unreferenced and replaced by > the new one, as will be the accompanying fields (i.e. data and size). > The rest isn't touched at all. > > This change will enable us to use only one packet in the bitstream > filters that rely on CBS. > > This commit also updates the documentation of ff_cbs_write_extradata > and ff_cbs_write_packet (to better describe existing behaviour and in > the latter case to also describe the new behaviour). > > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > --- > I could also have made it unref the packet's buffer at the beginning; > this would have the advantage that the packet's buffer would be freed > after the units have been rewritten (if they are rewritten) and after > the fragment's buffer has been unreferenced, so that maximum memory > consumption would decrease. It would also be in line with all current > callers of ff_cbs_write_packet, but maybe it wouldn't be what a future > caller wants. What do you think? > libavcodec/cbs.c | 3 ++- > libavcodec/cbs.h | 10 +++++++++- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/cbs.c b/libavcodec/cbs.c > index 0260ba6f67..47679eca1b 100644 > --- a/libavcodec/cbs.c > +++ b/libavcodec/cbs.c > @@ -357,7 +357,8 @@ int ff_cbs_write_packet(CodedBitstreamContext *ctx, > if (!buf) > return AVERROR(ENOMEM); > > - av_init_packet(pkt); > + av_buffer_unref(&pkt->buf);
You should probably unref the packet, not just the AVBufferRef. > + > pkt->buf = buf; > pkt->data = frag->data; > pkt->size = frag->data_size; > diff --git a/libavcodec/cbs.h b/libavcodec/cbs.h > index 967dcd1468..5260a39c63 100644 > --- a/libavcodec/cbs.h > +++ b/libavcodec/cbs.h > @@ -297,7 +297,8 @@ int ff_cbs_write_fragment_data(CodedBitstreamContext *ctx, > /** > * Write the bitstream of a fragment to the extradata in codec parameters. > * > - * This replaces any existing extradata in the structure. > + * Modifies context and fragment as ff_cbs_write_fragment_data does and > + * replaces any existing extradata in the structure. > */ > int ff_cbs_write_extradata(CodedBitstreamContext *ctx, > AVCodecParameters *par, > @@ -305,6 +306,13 @@ int ff_cbs_write_extradata(CodedBitstreamContext *ctx, > > /** > * Write the bitstream of a fragment to a packet. > + * > + * Modifies context and fragment as ff_cbs_write_fragment_data does. > + * > + * On success, the packet's buf is unreferenced and its buf, data and > + * size fields are set to the corresponding values from the newly updated > + * fragment; other fields are not touched. On failure, the packet is not > + * touched at all. > */ > int ff_cbs_write_packet(CodedBitstreamContext *ctx, > AVPacket *pkt, > _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
