diff mbox

[FFmpeg-devel,v2] ffmpeg: count packets when queued

Message ID 20170502073946.3243-1-mfcc64@gmail.com
State Accepted
Commit c4be288fdbe1993110f1abd28ea57587cb2bc221
Headers show

Commit Message

Muhammad Faiz May 2, 2017, 7:39 a.m. UTC 
Because write_packet() fakely writes packets to muxer by queueing
them when muxer hasn't been initialized, it should also increment
frame_number fakely.
This is required because code in do_streamcopy() rely on
frame_number.

Should fix Ticket6227

Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
---
 ffmpeg.c | 38 ++++++++++++++++++++------------------
 1 file changed, 20 insertions(+), 18 deletions(-)

Comments

Michael Niedermayer May 2, 2017, 5:58 p.m. UTC | #1 
On Tue, May 02, 2017 at 02:39:46PM +0700, Muhammad Faiz wrote:
> Because write_packet() fakely writes packets to muxer by queueing
> them when muxer hasn't been initialized, it should also increment
> frame_number fakely.
> This is required because code in do_streamcopy() rely on
> frame_number.
> 
> Should fix Ticket6227
> 
> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
> ---
>  ffmpeg.c | 38 ++++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 18 deletions(-)

probably ok

i think in the long run all this queeuing stuff should be moved into
libavformat (there are already queues in there as well) and
not require logic in the user apps, thats off topic though

Also, is there an simple testcase ? if so please add it to fate

thx

[...]
Muhammad Faiz May 2, 2017, 7:14 p.m. UTC | #2 
On Wed, May 3, 2017 at 12:58 AM, Michael Niedermayer
<michael@niedermayer.cc> wrote:
> On Tue, May 02, 2017 at 02:39:46PM +0700, Muhammad Faiz wrote:
>> Because write_packet() fakely writes packets to muxer by queueing
>> them when muxer hasn't been initialized, it should also increment
>> frame_number fakely.
>> This is required because code in do_streamcopy() rely on
>> frame_number.
>>
>> Should fix Ticket6227
>>
>> Signed-off-by: Muhammad Faiz <mfcc64@gmail.com>
>> ---
>>  ffmpeg.c | 38 ++++++++++++++++++++------------------
>>  1 file changed, 20 insertions(+), 18 deletions(-)
>
> probably ok

Applied.

>
> i think in the long run all this queeuing stuff should be moved into
> libavformat (there are already queues in there as well) and
> not require logic in the user apps, thats off topic though
>
> Also, is there an simple testcase ? if so please add it to fate

No. Maybe testcase in ticket entry can be a candidate.

Thank's.
diff mbox

Patch

diff --git a/ffmpeg.c b/ffmpeg.c
index bf04a6c..e798d92 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -669,12 +669,28 @@  static void close_all_output_streams(OutputStream *ost, OSTFinished this_stream,
     }
 }
 
-static void write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost)
+static void write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost, int unqueue)
 {
     AVFormatContext *s = of->ctx;
     AVStream *st = ost->st;
     int ret;
 
+    /*
+     * Audio encoders may split the packets --  #frames in != #packets out.
+     * But there is no reordering, so we can limit the number of output packets
+     * by simply dropping them here.
+     * Counting encoded video frames needs to be done separately because of
+     * reordering, see do_video_out().
+     * Do not count the packet when unqueued because it has been counted when queued.
+     */
+    if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && ost->encoding_needed) && !unqueue) {
+        if (ost->frame_number >= ost->max_frames) {
+            av_packet_unref(pkt);
+            return;
+        }
+        ost->frame_number++;
+    }
+
     if (!of->header_written) {
         AVPacket tmp_pkt = {0};
         /* the muxer is not initialized yet, buffer the packet */
@@ -703,20 +719,6 @@  static void write_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost)
         (st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO && audio_sync_method < 0))
         pkt->pts = pkt->dts = AV_NOPTS_VALUE;
 
-    /*
-     * Audio encoders may split the packets --  #frames in != #packets out.
-     * But there is no reordering, so we can limit the number of output packets
-     * by simply dropping them here.
-     * Counting encoded video frames needs to be done separately because of
-     * reordering, see do_video_out()
-     */
-    if (!(st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO && ost->encoding_needed)) {
-        if (ost->frame_number >= ost->max_frames) {
-            av_packet_unref(pkt);
-            return;
-        }
-        ost->frame_number++;
-    }
     if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO) {
         int i;
         uint8_t *sd = av_packet_get_side_data(pkt, AV_PKT_DATA_QUALITY_STATS,
@@ -861,10 +863,10 @@  static void output_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost)
                     goto finish;
                 idx++;
             } else
-                write_packet(of, pkt, ost);
+                write_packet(of, pkt, ost, 0);
         }
     } else
-        write_packet(of, pkt, ost);
+        write_packet(of, pkt, ost, 0);
 
 finish:
     if (ret < 0 && ret != AVERROR_EOF) {
@@ -2972,7 +2974,7 @@  static int check_init_output_file(OutputFile *of, int file_index)
         while (av_fifo_size(ost->muxing_queue)) {
             AVPacket pkt;
             av_fifo_generic_read(ost->muxing_queue, &pkt, sizeof(pkt), NULL);
-            write_packet(of, &pkt, ost);
+            write_packet(of, &pkt, ost, 1);
         }
     }