[FFmpeg-devel,v2,7/8] avformat/utils: Remove unnecessary packet copies

Submitted by Andreas Rheinhardt on Aug. 19, 2019, 9:56 p.m.

Details

Message ID 20190819215624.49795-7-andreas.rheinhardt@gmail.com
State New
Headers show

Commit Message

Andreas Rheinhardt Aug. 19, 2019, 9:56 p.m.
Up until now, read_frame_internal in avformat/utils.c uses a spare
packet on the stack that serves no real purpose: At no point in this
function is there a need for another packet besides the packet destined
for output:
1. If the packet doesn't need a parser, but is output as is, the content
of the spare packet (that at this point contains a freshly read packet)
is simply copied into the output packet (via simple assignment, not
av_packet_move_ref, thereby confusing ownership).
2. If the packet needs parsing, the spare packet will be reset after
parsing and any packets resulting from the packet read will be put into
a packet list; the output packet is not used here at all.
3. If the stream should be discarded, the spare packet will be
unreferenced; the output packet is not used here at all.

Therefore the spare packet and the copies can be removed in priniple.
In practice, one more thing needs to be taken care of: If ff_read_packet
failed, this did not affect the output packet, now it does. This
potential problem is solved by making sure that ff_read_packet always
resets the output packet in case of errors.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
Side note: This skip_to_keyframe stuff which is touched in this commit
has been introduced in 4a95876f to be able to drop non-keyframes after
the parser in case the keyframes are wrongly marked in the file. But the
parser returns packets by putting them in the packet queue and not by
returning them via its pkt parameter, so that st->skip_to_keyframe will
never be unset and no packet will be dropped because of it for a stream
that gets parsed. It actually only works ("works" means that an error
message will be displayed for a broken file where the keyframes are not
correctly marked) for the file for which it was intended (the one from
issue 1003) by accident. Maybe it should be removed altogether?

 libavformat/utils.c | 51 ++++++++++++++++++++++-----------------------
 1 file changed, 25 insertions(+), 26 deletions(-)

Comments

Andriy Gelman Aug. 28, 2019, 3:15 p.m.
On Mon, 19. Aug 23:56, Andreas Rheinhardt wrote:
> Up until now, read_frame_internal in avformat/utils.c uses a spare
> packet on the stack that serves no real purpose: At no point in this
> function is there a need for another packet besides the packet destined
> for output:
> 1. If the packet doesn't need a parser, but is output as is, the content
> of the spare packet (that at this point contains a freshly read packet)
> is simply copied into the output packet (via simple assignment, not
> av_packet_move_ref, thereby confusing ownership).
> 2. If the packet needs parsing, the spare packet will be reset after
> parsing and any packets resulting from the packet read will be put into
> a packet list; the output packet is not used here at all.
> 3. If the stream should be discarded, the spare packet will be
> unreferenced; the output packet is not used here at all.
> 
> Therefore the spare packet and the copies can be removed in priniple.

typo: principle

> In practice, one more thing needs to be taken care of: If ff_read_packet
> failed, this did not affect the output packet, now it does. This
> potential problem is solved by making sure that ff_read_packet always
> resets the output packet in case of errors.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---

> Side note: This skip_to_keyframe stuff which is touched in this commit
> has been introduced in 4a95876f to be able to drop non-keyframes after
> the parser in case the keyframes are wrongly marked in the file. But the
> parser returns packets by putting them in the packet queue and not by
> returning them via its pkt parameter, so that st->skip_to_keyframe will
> never be unset and no packet will be dropped because of it for a stream
> that gets parsed. It actually only works ("works" means that an error
> message will be displayed for a broken file where the keyframes are not
> correctly marked) for the file for which it was intended (the one from
> issue 1003) by accident. Maybe it should be removed altogether?

I agree, when the parser is called, the skip_to_keyframe code is currently
skipped. 
Would it make sense for the skip_to_keyframe code to also be added after pkt 
is retrieved from ff_packet_list_get ? 

> 
>  libavformat/utils.c | 51 ++++++++++++++++++++++-----------------------
>  1 file changed, 25 insertions(+), 26 deletions(-)
> 
> diff --git a/libavformat/utils.c b/libavformat/utils.c
> index db0f53869f..d6d615b690 100644
> --- a/libavformat/utils.c
> +++ b/libavformat/utils.c
> @@ -830,6 +830,10 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>      int ret, i, err;
>      AVStream *st;
>  
> +    pkt->data = NULL;
> +    pkt->size = 0;
> +    av_init_packet(pkt);
> +
>      for (;;) {
>          AVPacketList *pktl = s->internal->raw_packet_buffer;
>          const AVPacket *pkt1;
> @@ -847,11 +851,11 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>              }
>          }
>  
> -        pkt->data = NULL;
> -        pkt->size = 0;
> -        av_init_packet(pkt);
>          ret = s->iformat->read_packet(s, pkt);
>          if (ret < 0) {
> +            pkt->data = NULL;
> +            pkt->size = 0;
> +            av_init_packet(pkt);
>              /* Some demuxers return FFERROR_REDO when they consume
>                 data and discard it (ignored streams, junk, extradata).
>                 We must re-call the demuxer to get the real packet. */
> @@ -1579,10 +1583,9 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>  
>      while (!got_packet && !s->internal->parse_queue) {
>          AVStream *st;
> -        AVPacket cur_pkt;
>  
>          /* read next packet */
> -        ret = ff_read_packet(s, &cur_pkt);
> +        ret = ff_read_packet(s, pkt);
>          if (ret < 0) {
>              if (ret == AVERROR(EAGAIN))
>                  return ret;
> @@ -1597,7 +1600,7 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>              break;
>          }
>          ret = 0;
> -        st  = s->streams[cur_pkt.stream_index];
> +        st  = s->streams[pkt->stream_index];
>  
>          /* update context if required */
>          if (st->internal->need_context_update) {
> @@ -1615,7 +1618,7 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>  
>              ret = avcodec_parameters_to_context(st->internal->avctx, st->codecpar);
>              if (ret < 0) {
> -                av_packet_unref(&cur_pkt);
> +                av_packet_unref(pkt);
>                  return ret;
>              }
>  
> @@ -1624,7 +1627,7 @@ FF_DISABLE_DEPRECATION_WARNINGS
>              /* update deprecated public codec context */
>              ret = avcodec_parameters_to_context(st->codec, st->codecpar);
>              if (ret < 0) {
> -                av_packet_unref(&cur_pkt);
> +                av_packet_unref(pkt);
>                  return ret;
>              }
>  FF_ENABLE_DEPRECATION_WARNINGS
> @@ -1633,23 +1636,23 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              st->internal->need_context_update = 0;
>          }
>  
> -        if (cur_pkt.pts != AV_NOPTS_VALUE &&
> -            cur_pkt.dts != AV_NOPTS_VALUE &&
> -            cur_pkt.pts < cur_pkt.dts) {
> +        if (pkt->pts != AV_NOPTS_VALUE &&
> +            pkt->dts != AV_NOPTS_VALUE &&
> +            pkt->pts < pkt->dts) {
>              av_log(s, AV_LOG_WARNING,
>                     "Invalid timestamps stream=%d, pts=%s, dts=%s, size=%d\n",
> -                   cur_pkt.stream_index,
> -                   av_ts2str(cur_pkt.pts),
> -                   av_ts2str(cur_pkt.dts),
> -                   cur_pkt.size);
> +                   pkt->stream_index,
> +                   av_ts2str(pkt->pts),
> +                   av_ts2str(pkt->dts),
> +                   pkt->size);
>          }
>          if (s->debug & FF_FDEBUG_TS)
>              av_log(s, AV_LOG_DEBUG,
>                     "ff_read_packet stream=%d, pts=%s, dts=%s, size=%d, duration=%"PRId64", flags=%d\n",
> -                   cur_pkt.stream_index,
> -                   av_ts2str(cur_pkt.pts),
> -                   av_ts2str(cur_pkt.dts),
> -                   cur_pkt.size, cur_pkt.duration, cur_pkt.flags);
> +                   pkt->stream_index,
> +                   av_ts2str(pkt->pts),
> +                   av_ts2str(pkt->dts),
> +                   pkt->size, pkt->duration, pkt->flags);
>  
>          if (st->need_parsing && !st->parser && !(s->flags & AVFMT_FLAG_NOPARSE)) {
>              st->parser = av_parser_init(st->codecpar->codec_id);
> @@ -1669,7 +1672,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
>  
>          if (!st->need_parsing || !st->parser) {
>              /* no parsing needed: we just output the packet as is */
> -            *pkt = cur_pkt;
>              compute_pkt_fields(s, st, NULL, pkt, AV_NOPTS_VALUE, AV_NOPTS_VALUE);
>              if ((s->iformat->flags & AVFMT_GENERIC_INDEX) &&
>                  (pkt->flags & AV_PKT_FLAG_KEY) && pkt->dts != AV_NOPTS_VALUE) {
> @@ -1679,7 +1681,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              }
>              got_packet = 1;
>          } else if (st->discard < AVDISCARD_ALL) {
> -            if ((ret = parse_packet(s, &cur_pkt, cur_pkt.stream_index)) < 0)
> +            if ((ret = parse_packet(s, pkt, pkt->stream_index)) < 0)
>                  return ret;
>              st->codecpar->sample_rate = st->internal->avctx->sample_rate;
>              st->codecpar->bit_rate = st->internal->avctx->bit_rate;
> @@ -1688,15 +1690,12 @@ FF_ENABLE_DEPRECATION_WARNINGS
>              st->codecpar->codec_id = st->internal->avctx->codec_id;
>          } else {
>              /* free packet */
> -            av_packet_unref(&cur_pkt);
> +            av_packet_unref(pkt);
>          }
>          if (pkt->flags & AV_PKT_FLAG_KEY)
>              st->skip_to_keyframe = 0;
>          if (st->skip_to_keyframe) {
> -            av_packet_unref(&cur_pkt);
> -            if (got_packet) {
> -                *pkt = cur_pkt;
> -            }
> +            av_packet_unref(pkt);
>              got_packet = 0;
>          }
>      }

I went over all the commits in this patchset. They look good to me. 
(I had a minor comment on Fix memleaks II).

Andriy
Andreas Rheinhardt Sept. 9, 2019, 10:16 p.m.
Andriy Gelman:
> On Mon, 19. Aug 23:56, Andreas Rheinhardt wrote:
>> Up until now, read_frame_internal in avformat/utils.c uses a spare
>> packet on the stack that serves no real purpose: At no point in this
>> function is there a need for another packet besides the packet destined
>> for output:
>> 1. If the packet doesn't need a parser, but is output as is, the content
>> of the spare packet (that at this point contains a freshly read packet)
>> is simply copied into the output packet (via simple assignment, not
>> av_packet_move_ref, thereby confusing ownership).
>> 2. If the packet needs parsing, the spare packet will be reset after
>> parsing and any packets resulting from the packet read will be put into
>> a packet list; the output packet is not used here at all.
>> 3. If the stream should be discarded, the spare packet will be
>> unreferenced; the output packet is not used here at all.
>>
>> Therefore the spare packet and the copies can be removed in priniple.
> 
> typo: principle

Thanks. New patch coming in a minute.
> 
>> In practice, one more thing needs to be taken care of: If ff_read_packet
>> failed, this did not affect the output packet, now it does. This
>> potential problem is solved by making sure that ff_read_packet always
>> resets the output packet in case of errors.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
>> ---
> 
>> Side note: This skip_to_keyframe stuff which is touched in this commit
>> has been introduced in 4a95876f to be able to drop non-keyframes after
>> the parser in case the keyframes are wrongly marked in the file. But the
>> parser returns packets by putting them in the packet queue and not by
>> returning them via its pkt parameter, so that st->skip_to_keyframe will
>> never be unset and no packet will be dropped because of it for a stream
>> that gets parsed. It actually only works ("works" means that an error
>> message will be displayed for a broken file where the keyframes are not
>> correctly marked) for the file for which it was intended (the one from
>> issue 1003) by accident. Maybe it should be removed altogether?
> 
> I agree, when the parser is called, the skip_to_keyframe code is currently
> skipped. 
> Would it make sense for the skip_to_keyframe code to also be added after pkt 
> is retrieved from ff_packet_list_get ? 
> 
No. The correct place would be in parse_packet before the packet is
added to the list of parsed packets (otherwise, if all packets on said
list are skipped, one would have to goto to the loop again so that
further packets are read and checked.

>>
>>  libavformat/utils.c | 51 ++++++++++++++++++++++-----------------------
>>  1 file changed, 25 insertions(+), 26 deletions(-)
>>
>> diff --git a/libavformat/utils.c b/libavformat/utils.c
>> index db0f53869f..d6d615b690 100644
>> --- a/libavformat/utils.c
>> +++ b/libavformat/utils.c
>> @@ -830,6 +830,10 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>>      int ret, i, err;
>>      AVStream *st;
>>  
>> +    pkt->data = NULL;
>> +    pkt->size = 0;
>> +    av_init_packet(pkt);
>> +
>>      for (;;) {
>>          AVPacketList *pktl = s->internal->raw_packet_buffer;
>>          const AVPacket *pkt1;
>> @@ -847,11 +851,11 @@ int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
>>              }
>>          }
>>  
>> -        pkt->data = NULL;
>> -        pkt->size = 0;
>> -        av_init_packet(pkt);
>>          ret = s->iformat->read_packet(s, pkt);
>>          if (ret < 0) {
>> +            pkt->data = NULL;
>> +            pkt->size = 0;
>> +            av_init_packet(pkt);
>>              /* Some demuxers return FFERROR_REDO when they consume
>>                 data and discard it (ignored streams, junk, extradata).
>>                 We must re-call the demuxer to get the real packet. */
>> @@ -1579,10 +1583,9 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>>  
>>      while (!got_packet && !s->internal->parse_queue) {
>>          AVStream *st;
>> -        AVPacket cur_pkt;
>>  
>>          /* read next packet */
>> -        ret = ff_read_packet(s, &cur_pkt);
>> +        ret = ff_read_packet(s, pkt);
>>          if (ret < 0) {
>>              if (ret == AVERROR(EAGAIN))
>>                  return ret;
>> @@ -1597,7 +1600,7 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>>              break;
>>          }
>>          ret = 0;
>> -        st  = s->streams[cur_pkt.stream_index];
>> +        st  = s->streams[pkt->stream_index];
>>  
>>          /* update context if required */
>>          if (st->internal->need_context_update) {
>> @@ -1615,7 +1618,7 @@ static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
>>  
>>              ret = avcodec_parameters_to_context(st->internal->avctx, st->codecpar);
>>              if (ret < 0) {
>> -                av_packet_unref(&cur_pkt);
>> +                av_packet_unref(pkt);
>>                  return ret;
>>              }
>>  
>> @@ -1624,7 +1627,7 @@ FF_DISABLE_DEPRECATION_WARNINGS
>>              /* update deprecated public codec context */
>>              ret = avcodec_parameters_to_context(st->codec, st->codecpar);
>>              if (ret < 0) {
>> -                av_packet_unref(&cur_pkt);
>> +                av_packet_unref(pkt);
>>                  return ret;
>>              }
>>  FF_ENABLE_DEPRECATION_WARNINGS
>> @@ -1633,23 +1636,23 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>              st->internal->need_context_update = 0;
>>          }
>>  
>> -        if (cur_pkt.pts != AV_NOPTS_VALUE &&
>> -            cur_pkt.dts != AV_NOPTS_VALUE &&
>> -            cur_pkt.pts < cur_pkt.dts) {
>> +        if (pkt->pts != AV_NOPTS_VALUE &&
>> +            pkt->dts != AV_NOPTS_VALUE &&
>> +            pkt->pts < pkt->dts) {
>>              av_log(s, AV_LOG_WARNING,
>>                     "Invalid timestamps stream=%d, pts=%s, dts=%s, size=%d\n",
>> -                   cur_pkt.stream_index,
>> -                   av_ts2str(cur_pkt.pts),
>> -                   av_ts2str(cur_pkt.dts),
>> -                   cur_pkt.size);
>> +                   pkt->stream_index,
>> +                   av_ts2str(pkt->pts),
>> +                   av_ts2str(pkt->dts),
>> +                   pkt->size);
>>          }
>>          if (s->debug & FF_FDEBUG_TS)
>>              av_log(s, AV_LOG_DEBUG,
>>                     "ff_read_packet stream=%d, pts=%s, dts=%s, size=%d, duration=%"PRId64", flags=%d\n",
>> -                   cur_pkt.stream_index,
>> -                   av_ts2str(cur_pkt.pts),
>> -                   av_ts2str(cur_pkt.dts),
>> -                   cur_pkt.size, cur_pkt.duration, cur_pkt.flags);
>> +                   pkt->stream_index,
>> +                   av_ts2str(pkt->pts),
>> +                   av_ts2str(pkt->dts),
>> +                   pkt->size, pkt->duration, pkt->flags);
>>  
>>          if (st->need_parsing && !st->parser && !(s->flags & AVFMT_FLAG_NOPARSE)) {
>>              st->parser = av_parser_init(st->codecpar->codec_id);
>> @@ -1669,7 +1672,6 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>  
>>          if (!st->need_parsing || !st->parser) {
>>              /* no parsing needed: we just output the packet as is */
>> -            *pkt = cur_pkt;
>>              compute_pkt_fields(s, st, NULL, pkt, AV_NOPTS_VALUE, AV_NOPTS_VALUE);
>>              if ((s->iformat->flags & AVFMT_GENERIC_INDEX) &&
>>                  (pkt->flags & AV_PKT_FLAG_KEY) && pkt->dts != AV_NOPTS_VALUE) {
>> @@ -1679,7 +1681,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>              }
>>              got_packet = 1;
>>          } else if (st->discard < AVDISCARD_ALL) {
>> -            if ((ret = parse_packet(s, &cur_pkt, cur_pkt.stream_index)) < 0)
>> +            if ((ret = parse_packet(s, pkt, pkt->stream_index)) < 0)
>>                  return ret;
>>              st->codecpar->sample_rate = st->internal->avctx->sample_rate;
>>              st->codecpar->bit_rate = st->internal->avctx->bit_rate;
>> @@ -1688,15 +1690,12 @@ FF_ENABLE_DEPRECATION_WARNINGS
>>              st->codecpar->codec_id = st->internal->avctx->codec_id;
>>          } else {
>>              /* free packet */
>> -            av_packet_unref(&cur_pkt);
>> +            av_packet_unref(pkt);
>>          }
>>          if (pkt->flags & AV_PKT_FLAG_KEY)
>>              st->skip_to_keyframe = 0;
>>          if (st->skip_to_keyframe) {
>> -            av_packet_unref(&cur_pkt);
>> -            if (got_packet) {
>> -                *pkt = cur_pkt;
>> -            }
>> +            av_packet_unref(pkt);
>>              got_packet = 0;
>>          }
>>      }
> 
> I went over all the commits in this patchset. They look good to me. 

Good to hear. Thanks.

> (I had a minor comment on Fix memleaks II).
> 
> Andriy
> 
- Andreas

Patch hide | download patch | download mbox

diff --git a/libavformat/utils.c b/libavformat/utils.c
index db0f53869f..d6d615b690 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -830,6 +830,10 @@  int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
     int ret, i, err;
     AVStream *st;
 
+    pkt->data = NULL;
+    pkt->size = 0;
+    av_init_packet(pkt);
+
     for (;;) {
         AVPacketList *pktl = s->internal->raw_packet_buffer;
         const AVPacket *pkt1;
@@ -847,11 +851,11 @@  int ff_read_packet(AVFormatContext *s, AVPacket *pkt)
             }
         }
 
-        pkt->data = NULL;
-        pkt->size = 0;
-        av_init_packet(pkt);
         ret = s->iformat->read_packet(s, pkt);
         if (ret < 0) {
+            pkt->data = NULL;
+            pkt->size = 0;
+            av_init_packet(pkt);
             /* Some demuxers return FFERROR_REDO when they consume
                data and discard it (ignored streams, junk, extradata).
                We must re-call the demuxer to get the real packet. */
@@ -1579,10 +1583,9 @@  static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
 
     while (!got_packet && !s->internal->parse_queue) {
         AVStream *st;
-        AVPacket cur_pkt;
 
         /* read next packet */
-        ret = ff_read_packet(s, &cur_pkt);
+        ret = ff_read_packet(s, pkt);
         if (ret < 0) {
             if (ret == AVERROR(EAGAIN))
                 return ret;
@@ -1597,7 +1600,7 @@  static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
             break;
         }
         ret = 0;
-        st  = s->streams[cur_pkt.stream_index];
+        st  = s->streams[pkt->stream_index];
 
         /* update context if required */
         if (st->internal->need_context_update) {
@@ -1615,7 +1618,7 @@  static int read_frame_internal(AVFormatContext *s, AVPacket *pkt)
 
             ret = avcodec_parameters_to_context(st->internal->avctx, st->codecpar);
             if (ret < 0) {
-                av_packet_unref(&cur_pkt);
+                av_packet_unref(pkt);
                 return ret;
             }
 
@@ -1624,7 +1627,7 @@  FF_DISABLE_DEPRECATION_WARNINGS
             /* update deprecated public codec context */
             ret = avcodec_parameters_to_context(st->codec, st->codecpar);
             if (ret < 0) {
-                av_packet_unref(&cur_pkt);
+                av_packet_unref(pkt);
                 return ret;
             }
 FF_ENABLE_DEPRECATION_WARNINGS
@@ -1633,23 +1636,23 @@  FF_ENABLE_DEPRECATION_WARNINGS
             st->internal->need_context_update = 0;
         }
 
-        if (cur_pkt.pts != AV_NOPTS_VALUE &&
-            cur_pkt.dts != AV_NOPTS_VALUE &&
-            cur_pkt.pts < cur_pkt.dts) {
+        if (pkt->pts != AV_NOPTS_VALUE &&
+            pkt->dts != AV_NOPTS_VALUE &&
+            pkt->pts < pkt->dts) {
             av_log(s, AV_LOG_WARNING,
                    "Invalid timestamps stream=%d, pts=%s, dts=%s, size=%d\n",
-                   cur_pkt.stream_index,
-                   av_ts2str(cur_pkt.pts),
-                   av_ts2str(cur_pkt.dts),
-                   cur_pkt.size);
+                   pkt->stream_index,
+                   av_ts2str(pkt->pts),
+                   av_ts2str(pkt->dts),
+                   pkt->size);
         }
         if (s->debug & FF_FDEBUG_TS)
             av_log(s, AV_LOG_DEBUG,
                    "ff_read_packet stream=%d, pts=%s, dts=%s, size=%d, duration=%"PRId64", flags=%d\n",
-                   cur_pkt.stream_index,
-                   av_ts2str(cur_pkt.pts),
-                   av_ts2str(cur_pkt.dts),
-                   cur_pkt.size, cur_pkt.duration, cur_pkt.flags);
+                   pkt->stream_index,
+                   av_ts2str(pkt->pts),
+                   av_ts2str(pkt->dts),
+                   pkt->size, pkt->duration, pkt->flags);
 
         if (st->need_parsing && !st->parser && !(s->flags & AVFMT_FLAG_NOPARSE)) {
             st->parser = av_parser_init(st->codecpar->codec_id);
@@ -1669,7 +1672,6 @@  FF_ENABLE_DEPRECATION_WARNINGS
 
         if (!st->need_parsing || !st->parser) {
             /* no parsing needed: we just output the packet as is */
-            *pkt = cur_pkt;
             compute_pkt_fields(s, st, NULL, pkt, AV_NOPTS_VALUE, AV_NOPTS_VALUE);
             if ((s->iformat->flags & AVFMT_GENERIC_INDEX) &&
                 (pkt->flags & AV_PKT_FLAG_KEY) && pkt->dts != AV_NOPTS_VALUE) {
@@ -1679,7 +1681,7 @@  FF_ENABLE_DEPRECATION_WARNINGS
             }
             got_packet = 1;
         } else if (st->discard < AVDISCARD_ALL) {
-            if ((ret = parse_packet(s, &cur_pkt, cur_pkt.stream_index)) < 0)
+            if ((ret = parse_packet(s, pkt, pkt->stream_index)) < 0)
                 return ret;
             st->codecpar->sample_rate = st->internal->avctx->sample_rate;
             st->codecpar->bit_rate = st->internal->avctx->bit_rate;
@@ -1688,15 +1690,12 @@  FF_ENABLE_DEPRECATION_WARNINGS
             st->codecpar->codec_id = st->internal->avctx->codec_id;
         } else {
             /* free packet */
-            av_packet_unref(&cur_pkt);
+            av_packet_unref(pkt);
         }
         if (pkt->flags & AV_PKT_FLAG_KEY)
             st->skip_to_keyframe = 0;
         if (st->skip_to_keyframe) {
-            av_packet_unref(&cur_pkt);
-            if (got_packet) {
-                *pkt = cur_pkt;
-            }
+            av_packet_unref(pkt);
             got_packet = 0;
         }
     }