| Message ID | 20181225204051.31082-1-barsnick@gmx.net |
|---|---|
| State | Superseded |
| Headers | show |
Moritz Barsnick (2018-12-25): > Signed-off-by: Moritz Barsnick <barsnick@gmx.net> > --- > This adds two double precision divisions per frame, which seems acceptable. > > I'm not sure scaling the limit by the factor is the correct idea. It feels > right regarding the discontinuities, but not according to the limit option's > description. I think it would be more logical to keep limit expressed as real time. This option is a safety net, it is not nimble enough to select time gaps to ignore in files that contain them. > > > doc/filters.texi | 4 ++++ > libavfilter/f_realtime.c | 7 +++++-- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/doc/filters.texi b/doc/filters.texi > index 65ce25bc18..60ebf2aa99 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -20829,6 +20829,10 @@ They accept the following options: > @item limit > Time limit for the pauses. Any pause longer than that will be considered > a timestamp discontinuity and reset the timer. Default is 2 seconds. > +@item speed > +Speed factor for processing. The value must be a float larger than zero. > +Values larger than 1.0 will speed up processing, smaller will slow it down. > +The @var{limit} is automatically adapted accordingly. Default is 1.0. > @end table > > @anchor{select} > diff --git a/libavfilter/f_realtime.c b/libavfilter/f_realtime.c > index 171c16aaaa..8d4fbf642b 100644 > --- a/libavfilter/f_realtime.c > +++ b/libavfilter/f_realtime.c > @@ -22,11 +22,13 @@ > #include "libavutil/time.h" > #include "avfilter.h" > #include "internal.h" > +#include <float.h> > > typedef struct RealtimeContext { > const AVClass *class; > int64_t delta; > int64_t limit; > + double speed; > unsigned inited; > } RealtimeContext; > > @@ -36,7 +38,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) > RealtimeContext *s = ctx->priv; > > if (frame->pts != AV_NOPTS_VALUE) { > - int64_t pts = av_rescale_q(frame->pts, inlink->time_base, AV_TIME_BASE_Q); > + int64_t pts = av_rescale_q(frame->pts, inlink->time_base, AV_TIME_BASE_Q) / s->speed; > int64_t now = av_gettime_relative(); > int64_t sleep = pts - now + s->delta; > if (!s->inited) { > @@ -44,7 +46,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) > sleep = 0; > s->delta = now - pts; > } > - if (sleep > s->limit || sleep < -s->limit) { > + if (sleep > s->limit / s->speed || sleep < -s->limit / s->speed) { I should have used FFABS(sleep); if this hunk stays, then better make the change to have only one division: if (FFABS(sleep) > s->limit / s->speed) > av_log(ctx, AV_LOG_WARNING, > "time discontinuity detected: %"PRIi64" us, resetting\n", > sleep); > @@ -65,6 +67,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) > #define FLAGS AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_FILTERING_PARAM > static const AVOption options[] = { > { "limit", "sleep time limit", OFFSET(limit), AV_OPT_TYPE_DURATION, { .i64 = 2000000 }, 0, INT64_MAX, FLAGS }, > + { "speed", "speed factor", OFFSET(speed), AV_OPT_TYPE_DOUBLE, { .dbl = 1.0 }, DBL_MIN, DBL_MAX, FLAGS }, > { NULL } > }; > LGTM apart from these nitpicks. Regards,
On Wed, Dec 26, 2018 at 18:37:42 +0100, Nicolas George wrote: > > I'm not sure scaling the limit by the factor is the correct idea. It feels > > right regarding the discontinuities, but not according to the limit option's > > description. > > I think it would be more logical to keep limit expressed as real time. > This option is a safety net, it is not nimble enough to select time gaps > to ignore in files that contain them. So the limit shall be in "real" realtime? In other words, if the max sleep time limit is 2 seconds (default), it shall stay there, even if a speed < 1 slowdown extends all sleep times? (The use case is, for example: Slowdown a 25 fps stream with 0.01, all sleep times will be around 4 seconds, and therefore be skipped, which is not quite intended. The user would need to adapt the limit explicitly.) > I should have used FFABS(sleep); if this hunk stays, then better make > the change to have only one division: > > if (FFABS(sleep) > s->limit / s->speed) I can do so. Who decides whether the hunks stays or goes? I'm indecisive. > LGTM apart from these nitpicks. Thanks, Moritz
Moritz Barsnick (2018-12-27): > So the limit shall be in "real" realtime? In other words, if the max > sleep time limit is 2 seconds (default), it shall stay there, even if a > speed < 1 slowdown extends all sleep times? That was my point, but it is only an opinion. > (The use case is, for example: Slowdown a 25 fps stream with 0.01, all > sleep times will be around 4 seconds, and therefore be skipped, which > is not quite intended. The user would need to adapt the limit > explicitly.) I had not realized you wanted to slow things down that much. > I can do so. Who decides whether the hunks stays or goes? I'm > indecisive. I think since you made the effort of implementing this, you get to decide that kind of fine details. The preference I have expressed is very minor, both solutions are ok with me. Regards,
On 12/25/18, Moritz Barsnick <barsnick@gmx.net> wrote: > Signed-off-by: Moritz Barsnick <barsnick@gmx.net> > --- > This adds two double precision divisions per frame, which seems acceptable. > > I'm not sure scaling the limit by the factor is the correct idea. It feels > right regarding the discontinuities, but not according to the limit option's > description. > What is status of this patch?
On Tue, Apr 30, 2019 at 22:13:13 +0200, Paul B Mahol wrote: > > This adds two double precision divisions per frame, which seems acceptable. > > What is status of this patch? While contemplating over it, I apparently got over it. V2 posted. Retained the automatic sleep adjustment. Thanks, Moritz
diff --git a/doc/filters.texi b/doc/filters.texi index 65ce25bc18..60ebf2aa99 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -20829,6 +20829,10 @@ They accept the following options: @item limit Time limit for the pauses. Any pause longer than that will be considered a timestamp discontinuity and reset the timer. Default is 2 seconds. +@item speed +Speed factor for processing. The value must be a float larger than zero. +Values larger than 1.0 will speed up processing, smaller will slow it down. +The @var{limit} is automatically adapted accordingly. Default is 1.0. @end table @anchor{select} diff --git a/libavfilter/f_realtime.c b/libavfilter/f_realtime.c index 171c16aaaa..8d4fbf642b 100644 --- a/libavfilter/f_realtime.c +++ b/libavfilter/f_realtime.c @@ -22,11 +22,13 @@ #include "libavutil/time.h" #include "avfilter.h" #include "internal.h" +#include <float.h> typedef struct RealtimeContext { const AVClass *class; int64_t delta; int64_t limit; + double speed; unsigned inited; } RealtimeContext; @@ -36,7 +38,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) RealtimeContext *s = ctx->priv; if (frame->pts != AV_NOPTS_VALUE) { - int64_t pts = av_rescale_q(frame->pts, inlink->time_base, AV_TIME_BASE_Q); + int64_t pts = av_rescale_q(frame->pts, inlink->time_base, AV_TIME_BASE_Q) / s->speed; int64_t now = av_gettime_relative(); int64_t sleep = pts - now + s->delta; if (!s->inited) { @@ -44,7 +46,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) sleep = 0; s->delta = now - pts; } - if (sleep > s->limit || sleep < -s->limit) { + if (sleep > s->limit / s->speed || sleep < -s->limit / s->speed) { av_log(ctx, AV_LOG_WARNING, "time discontinuity detected: %"PRIi64" us, resetting\n", sleep); @@ -65,6 +67,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *frame) #define FLAGS AV_OPT_FLAG_VIDEO_PARAM | AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_FILTERING_PARAM static const AVOption options[] = { { "limit", "sleep time limit", OFFSET(limit), AV_OPT_TYPE_DURATION, { .i64 = 2000000 }, 0, INT64_MAX, FLAGS }, + { "speed", "speed factor", OFFSET(speed), AV_OPT_TYPE_DOUBLE, { .dbl = 1.0 }, DBL_MIN, DBL_MAX, FLAGS }, { NULL } };
Signed-off-by: Moritz Barsnick <barsnick@gmx.net> --- This adds two double precision divisions per frame, which seems acceptable. I'm not sure scaling the limit by the factor is the correct idea. It feels right regarding the discontinuities, but not according to the limit option's description. doc/filters.texi | 4 ++++ libavfilter/f_realtime.c | 7 +++++-- 2 files changed, 9 insertions(+), 2 deletions(-)