[FFmpeg-devel] [PATCH] ffmpeg: implement -force_key_frames_expr option
Stefano Sabatini
stefasab at gmail.com
Fri Dec 14 00:56:42 CET 2012
On date Thursday 2012-12-13 16:53:57 +0100, Nicolas George encoded:
> Le tridi 23 frimaire, an CCXXI, Stefano Sabatini a écrit :
> > ---
> > doc/ffmpeg.texi | 20 ++++++++++++++++++++
> > ffmpeg.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
> > ffmpeg.h | 15 +++++++++++++++
> > ffmpeg_opt.c | 7 +++++++
> > 4 files changed, 84 insertions(+), 2 deletions(-)
> >
> > diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> > index b8f6930..ff14e04 100644
> > --- a/doc/ffmpeg.texi
> > +++ b/doc/ffmpeg.texi
> > @@ -551,12 +551,32 @@ Force video tag/fourcc. This is an alias for @code{-tag:v}.
> > Show QP histogram
> > @item -vbsf @var{bitstream_filter}
> > Deprecated see -bsf
> > +
> > @item -force_key_frames[:@var{stream_specifier}] @var{time}[, at var{time}...] (@emph{output,per-stream})
> > Force key frames at the specified timestamps, more precisely at the first
> > frames after each specified time.
> > This option can be useful to ensure that a seek point is present at a
> > chapter mark or any other designated place in the output file.
> > The timestamps must be specified in ascending order.
> > +See also the @option{force_key_frames_expr} option.
> > +
> > + at item -force_key_frames_expr[:@var{stream_specifier}] @var{expr} (@emph{output,per-stream})
> > +Force key frames at the time specified by the expression in
>
> ... at the times specified by the expression. The expression will be
> evaluated repeatedly to get the time for each forced keyframe.
>
> > + at var{expr}, more precisely at the first frames after each specified
> > +time. The expression can contain the following constants:
> > +
> > + at table @option
> > + at item n
> > +The count of the next match, starting from 0.
>
> Number of already forced keyframes.
Changed.
> > + at item prev_t
> > +The time of the last forced key frame, it is @code{NAN} when no
> ^
> (value of the last evaluation of the expression)
No, this is not the same thing.
>
> > +keyframe was still forced.
>
> ... was forced yet.
>
> > + at end table
> > +
> > +For example you can specify the expression @code{n*5} to force a key
> > +frame every 5 seconds, or @code{prev_t+5} to specify a key frame after
> > +the last one forced.
> > +See also the @option{force_key_frames} option.
>
> Please add a note that forcing too many keyframes is very harmful for the
> lookahead algorithms (at least for x264): using fixed-GOP options or similar
> would be more efficient.
Done.
> > @item -copyinkf[:@var{stream_specifier}] (@emph{output,per-stream})
> > When doing stream copy, copy also non-key frames found at the
> > diff --git a/ffmpeg.c b/ffmpeg.c
> > index 956f5b6..7985051 100644
> > --- a/ffmpeg.c
> > +++ b/ffmpeg.c
> > @@ -109,6 +109,13 @@ const int program_birth_year = 2000;
> >
> > static FILE *vstats_file;
> >
> > +const char *const forced_keyframes_const_names[] = {
> > + "n",
> > + "prev_t",
>
> > + "next_t",
>
> Undocumented?
Changed to a context variable.
>
> > + NULL
> > +};
> > +
> > static void do_video_stats(OutputStream *ost, int frame_size);
> > static int64_t getutime(void);
> >
> > @@ -435,6 +442,8 @@ static void exit_program(void)
> > avcodec_free_frame(&output_streams[i]->filtered_frame);
> >
> > av_freep(&output_streams[i]->forced_keyframes);
> > + av_freep(&output_streams[i]->forced_keyframes_expr);
> > + av_expr_free(output_streams[i]->forced_keyframes_pexpr);
> > av_freep(&output_streams[i]->avfilter);
> > av_freep(&output_streams[i]->logfile_prefix);
> > av_freep(&output_streams[i]);
> > @@ -872,8 +881,9 @@ static void do_video_out(AVFormatContext *s,
> > video_size += pkt.size;
> > write_frame(s, &pkt, ost);
> > } else {
> > - int got_packet;
> > + int got_packet, forced_keyframe = 0;
> > AVFrame big_picture;
> > + double pts_time;
> >
> > big_picture = *in_picture;
> > /* better than nothing: use input picture interlaced
> > @@ -897,11 +907,26 @@ static void do_video_out(AVFormatContext *s,
> > big_picture.quality = ost->st->codec->global_quality;
> > if (!enc->me_threshold)
> > big_picture.pict_type = 0;
> > +
> > + pts_time = big_picture.pts != AV_NOPTS_VALUE ?
> > + big_picture.pts * av_q2d(enc->time_base) : NAN;
>
> Indentation is unusual.
It is what emacs likes (and please let's not fight over silly
indentation rules).
>
> > if (ost->forced_kf_index < ost->forced_kf_count &&
> > big_picture.pts >= ost->forced_kf_pts[ost->forced_kf_index]) {
> > - big_picture.pict_type = AV_PICTURE_TYPE_I;
> > ost->forced_kf_index++;
> > + forced_keyframe = 1;
> > + } else if (pts_time != AV_NOPTS_VALUE &&
> > + pts_time >= ost->forced_keyframes_expr_const_values[FKF_NEXT_T]) {
> > + ost->forced_keyframes_expr_const_values[FKF_N] += 1;
> > + ost->forced_keyframes_expr_const_values[FKF_PREV_T] = pts_time;
> > + ost->forced_keyframes_expr_const_values[FKF_NEXT_T] =
> > + av_expr_eval(ost->forced_keyframes_pexpr, ost->forced_keyframes_expr_const_values, NULL);
> > + forced_keyframe = 1;
> > + }
> > + if (forced_keyframe) {
> > + big_picture.pict_type = AV_PICTURE_TYPE_I;
> > + av_log(NULL, AV_LOG_DEBUG, "Forced keyframe at time %f\n", pts_time);
> > }
> > +
> > update_benchmark(NULL);
> > ret = avcodec_encode_video2(enc, &pkt, &big_picture, &got_packet);
> > update_benchmark("encode_video %d.%d", ost->file_index, ost->index);
> > @@ -2226,9 +2251,24 @@ static int transcode_init(void)
> > codec->bits_per_raw_sample = frame_bits_per_raw_sample;
> > }
> >
> > + if (ost->forced_keyframes && ost->forced_keyframes_expr) {
> > + av_log(NULL, AV_LOG_ERROR,
> > + "forced_keyframes and forced_keyframes_expr are incompatible, select just one\n");
> > + return AVERROR(EINVAL);
> > + }
> > if (ost->forced_keyframes)
> > parse_forced_key_frames(ost->forced_keyframes, ost,
> > ost->st->codec);
> > + if (ost->forced_keyframes_expr) {
> > + ret = av_expr_parse(&ost->forced_keyframes_pexpr, ost->forced_keyframes_expr,
> > + forced_keyframes_const_names, NULL, NULL, NULL, NULL, 0, NULL);
> > + if (ret < 0)
> > + return ret;
> > + ost->forced_keyframes_expr_const_values[FKF_N] = 0;
> > + ost->forced_keyframes_expr_const_values[FKF_PREV_T] = NAN;
> > + ost->forced_keyframes_expr_const_values[FKF_NEXT_T] =
> > + av_expr_eval(ost->forced_keyframes_pexpr, ost->forced_keyframes_expr_const_values, NULL);
> > + }
>
> Possible alternate implementation: -force_key_frames expr:prev_t+5.
>
> It has the advantage of not requiring all the code for new options, you just
> have to add a test in parse_forced_key_frames(); that would make the code
> much simpler I believe. And you do not have to worry about the user giving
> incompatible options.
Changed this way, it is slightly simpler. I don't know if it would
make sense to specify both expression and list (in this case it would
also make sense to split the options), probably not.
[...]
Patch updated.
--
FFmpeg = Friendly and Fucking Martial Powered Efficient Guru
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-ffmpeg-implement-force_key_frames-expression-evaluti.patch
Type: text/x-diff
Size: 7604 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121214/e1821ec6/attachment.bin>
More information about the ffmpeg-devel
mailing list