[FFmpeg-devel] [PATCH] ffmpeg: implement -force_key_frames_expr option
Michael Niedermayer
michaelni at gmx.at
Fri Dec 14 03:15:22 CET 2012
On Fri, Dec 14, 2012 at 12:56:42AM +0100, Stefano Sabatini wrote:
> 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
> doc/ffmpeg.texi | 38 ++++++++++++++++++++++++++++++++------
> ffmpeg.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
> ffmpeg.h | 12 ++++++++++++
> 3 files changed, 84 insertions(+), 11 deletions(-)
> fd3db7ff91b99d41766f00dc9d65b4117801db31 0003-ffmpeg-implement-force_key_frames-expression-evaluti.patch
> From 36786e76ccad1a89037542d4596a04f373215c81 Mon Sep 17 00:00:00 2001
> From: Stefano Sabatini <stefasab at gmail.com>
> Date: Sun, 9 Dec 2012 18:40:22 +0100
> Subject: [PATCH] ffmpeg: implement -force_key_frames expression evalution
>
> ---
> doc/ffmpeg.texi | 38 ++++++++++++++++++++++++++++++++------
> ffmpeg.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
> ffmpeg.h | 12 ++++++++++++
> 3 files changed, 84 insertions(+), 11 deletions(-)
>
> diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi
> index b8f6930..0915025 100644
> --- a/doc/ffmpeg.texi
> +++ b/doc/ffmpeg.texi
> @@ -551,12 +551,38 @@ Force video tag/fourcc. This is an alias for @code{-tag:v}.
> Show QP histogram
> @item -vbsf @var{bitstream_filter}
> Deprecated see -bsf
> - at 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.
> +
> + at item -force_key_frames[:@var{stream_specifier}] (@var{time}[, at var{time}...])|expr:@var{expr} (@emph{output,per-stream})
> +Force key frames at the specified timestamps, more precisely at the
> +first frames after each specified time.
> +
> +If a list of times is specified as argument, the 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.
> +
> +If the argument is prefixed with @code{expr:}, the string @var{expr}
> +is interpreted like an expression and is evaluated repeatedly to get
> +the time of the next keyframe to force.
> +
> +The expression in @var{expr} can contain the following constants:
> + at table @option
> + at item n
> +the number of already forced keyframes
> + at item prev_t
> +the time of the last forced key frame, it is @code{NAN} when no
> +keyframe was forced yet
> + at end table
implementing this in ffmpeg.c makes the expression evaluation only
available to ffmpeg and not to other user applications, also the
expression evaluator only has a small number of input variables
available from the application.
in libavcodec it would have a much larger amount of information
available, like the scene change score or various rate control
parameters
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121214/01b48432/attachment.asc>
More information about the ffmpeg-devel
mailing list