[FFmpeg-devel] [PATCH v4 3/9] avformat/utils: Move the reference to the packet list
James Almer
jamrial at gmail.com
Thu Sep 26 00:27:13 EEST 2019
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.rheinhardt at 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++;
>
More information about the ffmpeg-devel
mailing list