| Message ID | 1541691321-12543-1-git-send-email-mypopydev@gmail.com |
|---|---|
| State | New |
| Headers | show |
2018-11-08 16:35 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: > Signed-off-by: Jun Zhao <mypopydev@gmail.com> > --- > fftools/ffmpeg.c | 62 > ++++++++++++++++++++++++----------------------------- > 1 files changed, 28 insertions(+), 34 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index da4259a..e989e7a 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -1193,33 +1193,27 @@ static void do_video_out(OutputFile *of, > } > ost->last_dropped = nb_frames == nb0_frames && next_picture; > > - /* duplicates frame if needed */ > - for (i = 0; i < nb_frames; i++) { > - AVFrame *in_picture; > - av_init_packet(&pkt); > - pkt.data = NULL; > - pkt.size = 0; > - > - if (i < nb0_frames && ost->last_frame) { > - in_picture = ost->last_frame; > - } else > - in_picture = next_picture; > + /* duplicates frame if needed */ > + for (i = 0; i < nb_frames; i++) { > + AVFrame *in_picture; > + int forced_keyframe = 0; > + double pts_time; > + av_init_packet(&pkt); > + pkt.data = NULL; > + pkt.size = 0; > > - if (!in_picture) > - return; > + if (i < nb0_frames && ost->last_frame) { > + in_picture = ost->last_frame; > + } else > + in_picture = next_picture; This patch seems to mix functional changes with re-indentation. The patch will be much easier to review if you move all re-indentation into the second patch. Carl Eugen
On Thu, Nov 08, 2018 at 11:35:20PM +0800, Jun Zhao wrote: > Signed-off-by: Jun Zhao <mypopydev@gmail.com> > --- > fftools/ffmpeg.c | 62 ++++++++++++++++++++++++----------------------------- > 1 files changed, 28 insertions(+), 34 deletions(-) The commit message "Refine the do_video_out." is too terse its not clear from it what is done, why it is done, how it is done thx [...]
On Thu, Nov 8, 2018 at 11:45 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > > 2018-11-08 16:35 GMT+01:00, Jun Zhao <mypopydev@gmail.com>: > > Signed-off-by: Jun Zhao <mypopydev@gmail.com> > > --- > > fftools/ffmpeg.c | 62 > > ++++++++++++++++++++++++----------------------------- > > 1 files changed, 28 insertions(+), 34 deletions(-) > > > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > > index da4259a..e989e7a 100644 > > --- a/fftools/ffmpeg.c > > +++ b/fftools/ffmpeg.c > > @@ -1193,33 +1193,27 @@ static void do_video_out(OutputFile *of, > > } > > ost->last_dropped = nb_frames == nb0_frames && next_picture; > > > > - /* duplicates frame if needed */ > > - for (i = 0; i < nb_frames; i++) { > > - AVFrame *in_picture; > > - av_init_packet(&pkt); > > - pkt.data = NULL; > > - pkt.size = 0; > > - > > - if (i < nb0_frames && ost->last_frame) { > > - in_picture = ost->last_frame; > > - } else > > - in_picture = next_picture; > > + /* duplicates frame if needed */ > > + for (i = 0; i < nb_frames; i++) { > > + AVFrame *in_picture; > > + int forced_keyframe = 0; > > + double pts_time; > > + av_init_packet(&pkt); > > + pkt.data = NULL; > > + pkt.size = 0; > > > > - if (!in_picture) > > - return; > > + if (i < nb0_frames && ost->last_frame) { > > + in_picture = ost->last_frame; > > + } else > > + in_picture = next_picture; > > This patch seems to mix functional changes with re-indentation. > The patch will be much easier to review if you move all re-indentation > into the second patch. > > Carl Eugen Will split the patch and move the re-indentataion to 2nd patch, Thanks
On Fri, Nov 9, 2018 at 4:24 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > > On Thu, Nov 08, 2018 at 11:35:20PM +0800, Jun Zhao wrote: > > Signed-off-by: Jun Zhao <mypopydev@gmail.com> > > --- > > fftools/ffmpeg.c | 62 ++++++++++++++++++++++++----------------------------- > > 1 files changed, 28 insertions(+), 34 deletions(-) > > The commit message "Refine the do_video_out." is too terse > its not clear from it what is done, why it is done, how it is done > > thx > The v2 patchset will give more detail information in commit message, thanks the review.
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index da4259a..e989e7a 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -1193,33 +1193,27 @@ static void do_video_out(OutputFile *of, } ost->last_dropped = nb_frames == nb0_frames && next_picture; - /* duplicates frame if needed */ - for (i = 0; i < nb_frames; i++) { - AVFrame *in_picture; - av_init_packet(&pkt); - pkt.data = NULL; - pkt.size = 0; - - if (i < nb0_frames && ost->last_frame) { - in_picture = ost->last_frame; - } else - in_picture = next_picture; + /* duplicates frame if needed */ + for (i = 0; i < nb_frames; i++) { + AVFrame *in_picture; + int forced_keyframe = 0; + double pts_time; + av_init_packet(&pkt); + pkt.data = NULL; + pkt.size = 0; - if (!in_picture) - return; + if (i < nb0_frames && ost->last_frame) { + in_picture = ost->last_frame; + } else + in_picture = next_picture; - in_picture->pts = ost->sync_opts; + if (!in_picture) + return; -#if 1 - if (!check_recording_time(ost)) -#else - if (ost->frame_number >= ost->max_frames) -#endif - return; + in_picture->pts = ost->sync_opts; - { - int forced_keyframe = 0; - double pts_time; + if (!check_recording_time(ost)) + return; if (enc->flags & (AV_CODEC_FLAG_INTERLACED_DCT | AV_CODEC_FLAG_INTERLACED_ME) && ost->top_field_first >= 0) @@ -1328,18 +1322,18 @@ static void do_video_out(OutputFile *of, fprintf(ost->logfile, "%s", enc->stats_out); } } - } - ost->sync_opts++; - /* - * For video, number of frames in == number of packets out. - * 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. - */ - ost->frame_number++; - if (vstats_filename && frame_size) - do_video_stats(ost, frame_size); - } + ost->sync_opts++; + /* + * For video, number of frames in == number of packets out. + * 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. + */ + ost->frame_number++; + + if (vstats_filename && frame_size) + do_video_stats(ost, frame_size); + } if (!ost->last_frame) ost->last_frame = av_frame_alloc();
Signed-off-by: Jun Zhao <mypopydev@gmail.com> --- fftools/ffmpeg.c | 62 ++++++++++++++++++++++++----------------------------- 1 files changed, 28 insertions(+), 34 deletions(-)