On 9/25/2019 9:05 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. > > Furthermore, the packet is always made refcounted so that its data is > really owned by the packet. This was already true for the current > callers.
Ah, my bad. When you said "I can add a commit doing this" i assumed you'd do it in a separate patch. I already committed the "v4 3/9" one. I'll apply the doxy changes and the av_packet_make_refcounted() call in a new patch in a moment. > > Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@gmail.com> > --- > libavformat/internal.h | 3 ++- > libavformat/utils.c | 37 ++++++++++++++++++++++--------------- > 2 files changed, 24 insertions(+), 16 deletions(-) > > diff --git a/libavformat/internal.h b/libavformat/internal.h > index 163587f416..a37404d823 100644 > --- a/libavformat/internal.h > +++ b/libavformat/internal.h > @@ -763,7 +763,8 @@ void ff_format_set_url(AVFormatContext *s, char *url); > * > * @param head List head element > * @param tail List tail element > - * @param pkt The packet being appended > + * @param pkt The packet being appended. It will be made refcounted > + * if it isn't already. > * @param flags Any combination of FF_PACKETLIST_FLAG_* flags > * @return 0 on success, negative AVERROR value on failure. On failure, > the list is unchanged > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 4657ba2642..9f8a5bfb63 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -460,10 +460,12 @@ 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; > + ret = av_packet_make_refcounted(pkt); > + if (ret < 0) { > + av_free(pktl); > + return ret; > + } > + av_packet_move_ref(&pktl->pkt, pkt); > } > > if (*packet_buffer) > @@ -835,6 +837,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]; > @@ -922,9 +925,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; > } > } > @@ -3032,8 +3036,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; > @@ -3525,7 +3529,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; > @@ -3588,7 +3592,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; > @@ -3707,6 +3711,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; > @@ -3800,14 +3805,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]; > @@ -3885,7 +3892,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) { > @@ -3922,7 +3929,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".