[FFmpeg-devel] [PATCH 2/3] avfilter/src_movie: Remove unnecessary secondary AVPacket

[Andreas Rheinhardt]

The movie and amovie filters currently use two packets. One of the two,
pkt0, is the owner of the returned packet; it is also the destination
packet for av_read_frame(). The other one pkt is initially (i.e. after
av_read_frame()) a copy of pkt0; copy means that the contents of both
are absolutely the same: They both point to the same AVBufferRef and the
same side data. This violation of the refcounted packet API is only
possible because pkt is not considered to own its data. Only pkt0 is
ever unreferenced.
The reason for pkt's existence seems to be historic:
The API used for decoding audio (namely avcodec_decode_audio4()) could
consume frames partially, i.e. it could return multiple frames for one
packet and therefore it returned how much of the input buffer had been
consumed. The caller was then supposed to update the packet's data and
size pointer to reflect this and call avcodec_decode_audio4() again with
the updated packet to get the next frame.
But before the introduction of refcounted AVPackets where knowledge and
responsibility about what to free lies with the underlying AVBuffer such
a procedure required a spare packet (or one would need to record the
original data and size fields separately to restore them before freeing
the packet; notice that this code has been written when AVPackets still
had a destruct field). But these times are long gone, so just remove the
secondary AVPacket.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
---
It seems that the avcodec_decode_audio4() lost the ability to output
multiple frames per packet in 061a0c14bb5767bca72e3a7227ca400de439ba09.
Am I right?

 libavfilter/src_movie.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/libavfilter/src_movie.c b/libavfilter/src_movie.c
index b9b940684d..75ac3bfaf6 100644
--- a/libavfilter/src_movie.c
+++ b/libavfilter/src_movie.c
@@ -71,7 +71,7 @@ typedef struct MovieContext {
 
     AVFormatContext *format_ctx;
     int eof;
-    AVPacket pkt, pkt0;
+    AVPacket pkt;
 
     int max_stream_index; /**< max stream # actually used for output */
     MovieStream *st; /**< array of all streams, one per output */
@@ -493,25 +493,21 @@ static int movie_push_frame(AVFilterContext *ctx, unsigned out_id)
             pkt->stream_index = movie->st[out_id].st->index;
             /* packet is already ready for flushing */
         } else {
-            ret = av_read_frame(movie->format_ctx, &movie->pkt0);
+            ret = av_read_frame(movie->format_ctx, pkt);
             if (ret < 0) {
-                *pkt = movie->pkt0;
                 if (ret == AVERROR_EOF) {
                     movie->eof = 1;
                     return 0; /* start flushing */
                 }
                 return ret;
             }
-            *pkt = movie->pkt0;
         }
     }
 
     pkt_out_id = pkt->stream_index > movie->max_stream_index ? -1 :
                  movie->out_index[pkt->stream_index];
     if (pkt_out_id < 0) {
-        av_packet_unref(&movie->pkt0);
-        pkt->size = 0; /* ready for next run */
-        pkt->data = NULL;
+        av_packet_unref(pkt);
         return 0;
     }
     st = &movie->st[pkt_out_id];
@@ -536,9 +532,7 @@ static int movie_push_frame(AVFilterContext *ctx, unsigned out_id)
     if (ret < 0) {
         av_log(ctx, AV_LOG_WARNING, "Decode error: %s\n", av_err2str(ret));
         av_frame_free(&frame);
-        av_packet_unref(&movie->pkt0);
-        movie->pkt.size = 0;
-        movie->pkt.data = NULL;
+        av_packet_unref(pkt);
         return 0;
     }
     if (!ret || st->st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO)
@@ -546,11 +540,8 @@ static int movie_push_frame(AVFilterContext *ctx, unsigned out_id)
 
     pkt->data += ret;
     pkt->size -= ret;
-    if (pkt->size <= 0) {
-        av_packet_unref(&movie->pkt0);
-        pkt->size = 0; /* ready for next run */
-        pkt->data = NULL;
-    }
+    if (pkt->size <= 0)
+        av_packet_unref(pkt);
     if (!got_frame) {
         if (!ret)
             st->done = 1;
-- 
2.20.1