| Message ID | 20211213152042.5900-23-anton@khirnov.net |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,01/24] ffmpeg: pass the muxer context explicitly to some functions | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Mon, Dec 13, 2021 at 04:20:41PM +0100, Anton Khirnov wrote: > It features in limiting the number of output frames (-frames option) and > currently can be incremented from two places: > - for video encoding (not streamcopy), in do_video_out() after each > successful avcodec_send_frame() call > - for all other cases, in of_submit_packet() > > Not only is this inconsistent and confusing, but also the special > treatment for video encoding is redundant, since > AVCodecContext.frame_count stores the exact same value (number of > successful avcodec_send_frame()) calls. > > Replace the use of OutputStream.frame_count in do_video_out() with > AVCodecContext.frame_count. Modify OutputStream.frame_count in the same > way for all streams. > --- > fftools/ffmpeg.c | 9 ++++----- > fftools/ffmpeg_mux.c | 19 +++++++------------ > 2 files changed, 11 insertions(+), 17 deletions(-) This results in differences one is: ./ffmpeg -i ~/tickets/4072/CoP\ 1\ 1\ 3\ -M.avi -vframes 2 -bitexact -vcodec huffyuv /tmp/4072-fic-old.avi -rw-r----- 1 michael michael 3572792 Dez 14 22:09 /tmp/4072-fic-new.avi -rw-r----- 1 michael michael 1908172 Dez 14 22:10 /tmp/4072-fic-old.avi I see other differences caused by patches today i still need to investigate what causes what thx [...]
Quoting Michael Niedermayer (2021-12-14 22:13:43) > On Mon, Dec 13, 2021 at 04:20:41PM +0100, Anton Khirnov wrote: > > It features in limiting the number of output frames (-frames option) and > > currently can be incremented from two places: > > - for video encoding (not streamcopy), in do_video_out() after each > > successful avcodec_send_frame() call > > - for all other cases, in of_submit_packet() > > > > Not only is this inconsistent and confusing, but also the special > > treatment for video encoding is redundant, since > > AVCodecContext.frame_count stores the exact same value (number of > > successful avcodec_send_frame()) calls. > > > > Replace the use of OutputStream.frame_count in do_video_out() with > > AVCodecContext.frame_count. Modify OutputStream.frame_count in the same > > way for all streams. > > --- > > fftools/ffmpeg.c | 9 ++++----- > > fftools/ffmpeg_mux.c | 19 +++++++------------ > > 2 files changed, 11 insertions(+), 17 deletions(-) > > This results in differences > one is: > ./ffmpeg -i ~/tickets/4072/CoP\ 1\ 1\ 3\ -M.avi -vframes 2 -bitexact -vcodec huffyuv /tmp/4072-fic-old.avi > > -rw-r----- 1 michael michael 3572792 Dez 14 22:09 /tmp/4072-fic-new.avi > -rw-r----- 1 michael michael 1908172 Dez 14 22:10 /tmp/4072-fic-old.avi > > I see other differences caused by patches today i still need to investigate > what causes what Fixing this will require larger restructuring first, so I am dropping this patch for now. Please let me know if the rest of the set breaks anything else.
On Wed, Dec 15, 2021 at 08:36:25PM +0100, Anton Khirnov wrote: > Quoting Michael Niedermayer (2021-12-14 22:13:43) > > On Mon, Dec 13, 2021 at 04:20:41PM +0100, Anton Khirnov wrote: > > > It features in limiting the number of output frames (-frames option) and > > > currently can be incremented from two places: > > > - for video encoding (not streamcopy), in do_video_out() after each > > > successful avcodec_send_frame() call > > > - for all other cases, in of_submit_packet() > > > > > > Not only is this inconsistent and confusing, but also the special > > > treatment for video encoding is redundant, since > > > AVCodecContext.frame_count stores the exact same value (number of > > > successful avcodec_send_frame()) calls. > > > > > > Replace the use of OutputStream.frame_count in do_video_out() with > > > AVCodecContext.frame_count. Modify OutputStream.frame_count in the same > > > way for all streams. > > > --- > > > fftools/ffmpeg.c | 9 ++++----- > > > fftools/ffmpeg_mux.c | 19 +++++++------------ > > > 2 files changed, 11 insertions(+), 17 deletions(-) > > > > This results in differences > > one is: > > ./ffmpeg -i ~/tickets/4072/CoP\ 1\ 1\ 3\ -M.avi -vframes 2 -bitexact -vcodec huffyuv /tmp/4072-fic-old.avi > > > > -rw-r----- 1 michael michael 3572792 Dez 14 22:09 /tmp/4072-fic-new.avi > > -rw-r----- 1 michael michael 1908172 Dez 14 22:10 /tmp/4072-fic-old.avi > > > > I see other differences caused by patches today i still need to investigate > > what causes what > > Fixing this will require larger restructuring first, so I am dropping > this patch for now. > > Please let me know if the rest of the set breaks anything else. all failures from this patch set where from this patch i think thx [...]
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 538601e440..aff3cc350e 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -1031,7 +1031,7 @@ static void do_video_out(OutputFile *of, switch (ost->vsync_method) { case VSYNC_VSCFR: - if (ost->frame_number == 0 && delta0 >= 0.5) { + if (enc->frame_number == 0 && delta0 >= 0.5) { av_log(NULL, AV_LOG_DEBUG, "Not duplicating %d initial frames\n", (int)lrintf(delta0)); delta = duration; delta0 = 0; @@ -1039,7 +1039,7 @@ static void do_video_out(OutputFile *of, } case VSYNC_CFR: // FIXME set to 0.5 after we fix some dts/pts bugs like in avidec.c - if (frame_drop_threshold && delta < frame_drop_threshold && ost->frame_number) { + if (frame_drop_threshold && delta < frame_drop_threshold && enc->frame_number) { nb_frames = 0; } else if (delta < -1.1) nb_frames = 0; @@ -1069,7 +1069,7 @@ static void do_video_out(OutputFile *of, * But there may be reordering, so we can't throw away frames on encoder * flush, we need to limit them here, before they go into encoder. */ - nb_frames = FFMIN(nb_frames, ost->max_frames - ost->frame_number); + nb_frames = FFMIN(nb_frames, ost->max_frames - enc->frame_number); nb0_frames = FFMIN(nb0_frames, nb_frames); memmove(ost->last_nb0_frames + 1, @@ -1081,7 +1081,7 @@ static void do_video_out(OutputFile *of, nb_frames_drop++; av_log(NULL, AV_LOG_VERBOSE, "*** dropping frame %d from stream %d at ts %"PRId64"\n", - ost->frame_number, ost->st->index, ost->last_frame->pts); + enc->frame_number, ost->st->index, ost->last_frame->pts); } if (nb_frames > (nb0_frames && ost->last_dropped) + (nb_frames > nb0_frames)) { if (nb_frames > dts_error_threshold * 30) { @@ -1222,7 +1222,6 @@ static void do_video_out(OutputFile *of, } } ost->sync_opts++; - ost->frame_number++; if (vstats_filename && frame_size) do_video_stats(ost, frame_size); diff --git a/fftools/ffmpeg_mux.c b/fftools/ffmpeg_mux.c index e97ec8ab93..c3666fbba1 100644 --- a/fftools/ffmpeg_mux.c +++ b/fftools/ffmpeg_mux.c @@ -200,23 +200,18 @@ static void write_packet(OutputFile *of, OutputStream *ost, AVPacket *pkt) void of_submit_packet(OutputFile *of, AVPacket *pkt, OutputStream *ost) { - 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(). + * Limit the number of output frames by dropping any excess packets. + * This should not trigger for video encoding, which due to possible + * reordering needs to be treated specially - 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 (ost->frame_number >= ost->max_frames) { + av_packet_unref(pkt); + return; } + ost->frame_number++; if (of->mux->header_written) { write_packet(of, ost, pkt);