On 9/20/2019 5:39 PM, Andreas Rheinhardt wrote: > Up until now, ff_packet_list_put had a flaw: When it moved a packet to > the list (meaning, when it ought to move the reference to the packet > list instead of creating a new one via av_packet_ref), it did not reset > the original packet, confusing the ownership of the data in the packet. > This has been done because some callers of this function were not > compatible with resetting the packet. > > This commit changes these callers and fixes this flaw. In order to > indicate that the ownership of the packet has moved to the packet list, > pointers to constant AVPackets are used whenever the target of the > pointer might already be owned by the packet list. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > --- > libavformat/utils.c | 32 +++++++++++++++++--------------- > 1 file changed, 17 insertions(+), 15 deletions(-) > > diff --git a/libavformat/utils.c b/libavformat/utils.c > index e348df3269..58e0db61da 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -460,10 +460,7 @@ int ff_packet_list_put(AVPacketList **packet_buffer, > return ret; > } > } else { > - // TODO: Adapt callers in this file so the line below can use > - // av_packet_move_ref() to effectively move the reference > - // to the list. > - pktl->pkt = *pkt; > + av_packet_move_ref(&pktl->pkt, pkt);
I think it would be a good idea ensuring the packet is reference counted here, to have more robust code and not depend on current callers having done it themselves. It's a matter of adding an av_packet_make_refcounted() call before this line, much like it's done in av_bsf_send_packet(). It's essentially a no-op when the packet is already reference counted. > } > > if (*packet_buffer) > @@ -835,6 +832,7 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt) > > for (;;) { > AVPacketList *pktl = s->internal->raw_packet_buffer; > + const AVPacket *pkt1; > > if (pktl) { > st = s->streams[pktl->pkt.stream_index]; > @@ -925,9 +923,10 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt) > av_packet_unref(pkt); > return err; > } > - s->internal->raw_packet_buffer_remaining_size -= pkt->size; > + pkt1 = &s->internal->raw_packet_buffer_end->pkt; > + s->internal->raw_packet_buffer_remaining_size -= pkt1->size; > > - if ((err = probe_codec(s, st, pkt)) < 0) > + if ((err = probe_codec(s, st, pkt1)) < 0) > return err; > } > } > @@ -3035,8 +3034,8 @@ static int has_codec_parameters(AVStream *st, const > char **errmsg_ptr) > } > > /* returns 1 or 0 if or if not decoded data was returned, or a negative > error */ > -static int try_decode_frame(AVFormatContext *s, AVStream *st, AVPacket > *avpkt, > - AVDictionary **options) > +static int try_decode_frame(AVFormatContext *s, AVStream *st, > + const AVPacket *avpkt, AVDictionary **options) > { > AVCodecContext *avctx = st->internal->avctx; > const AVCodec *codec; > @@ -3528,7 +3527,7 @@ fail: > return ret; > } > > -static int extract_extradata(AVStream *st, AVPacket *pkt) > +static int extract_extradata(AVStream *st, const AVPacket *pkt) > { > AVStreamInternal *sti = st->internal; > AVPacket *pkt_ref; > @@ -3591,7 +3590,7 @@ int avformat_find_stream_info(AVFormatContext *ic, > AVDictionary **options) > int64_t read_size; > AVStream *st; > AVCodecContext *avctx; > - AVPacket pkt1, *pkt; > + AVPacket pkt1; > int64_t old_offset = avio_tell(ic->pb); > // new streams might appear, no options for those > int orig_nb_streams = ic->nb_streams; > @@ -3710,6 +3709,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > > read_size = 0; > for (;;) { > + const AVPacket *pkt; > int analyzed_all_streams; > if (ff_check_interrupt(&ic->interrupt_callback)) { > ret = AVERROR_EXIT; > @@ -3803,14 +3803,16 @@ FF_ENABLE_DEPRECATION_WARNINGS > break; > } > > - pkt = &pkt1; > - > if (!(ic->flags & AVFMT_FLAG_NOBUFFER)) { > ret = ff_packet_list_put(&ic->internal->packet_buffer, > &ic->internal->packet_buffer_end, > - pkt, 0); > + &pkt1, 0); > if (ret < 0) > goto find_stream_info_err; > + > + pkt = &ic->internal->packet_buffer_end->pkt; > + } else { > + pkt = &pkt1; > } > > st = ic->streams[pkt->stream_index]; > @@ -3888,7 +3890,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > limit, > t, pkt->stream_index); > if (ic->flags & AVFMT_FLAG_NOBUFFER) > - av_packet_unref(pkt); > + av_packet_unref(&pkt1); > break; > } > if (pkt->duration) { > @@ -3925,7 +3927,7 @@ FF_ENABLE_DEPRECATION_WARNINGS > (options && i < orig_nb_streams) ? &options[i] : > NULL); > > if (ic->flags & AVFMT_FLAG_NOBUFFER) > - av_packet_unref(pkt); > + av_packet_unref(&pkt1); > > st->codec_info_nb_frames++; > count++; > _______________________________________________ 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".