On Sun, Nov 02, 2014 at 09:38:17PM -0800, Kevin Mitchell wrote: > Ok, I fixed the logic by removing round() and added an integer power > function to replace pow()
> doc/filters.texi | 5 ++
> libavfilter/version.h | 2 -
> libavfilter/vf_idet.c | 93
> +++++++++++++++++++++++++++++++++++++-------------
> libavfilter/vf_idet.h | 7 ++-
> 4 files changed, 80 insertions(+), 27 deletions(-)
> 236aa8f17a311e276c74e65d72fc9cf740a4e61f
> 0001-avfilter-vf_idet-add-a-half_life-option-for-statisti.patch
> From f66e072d477964ebcfe38eafe40d12030a91df14 Mon Sep 17 00:00:00 2001
> From: Kevin Mitchell <kevmi...@gmail.com>
> Date: Sun, 2 Nov 2014 21:30:51 -0800
> Subject: [PATCH 1/2] avfilter/vf_idet: add a "half_life" option for statistics
>
> This can be useful for videos in which the interlacing pattern changes.
> ---
> doc/filters.texi | 5 +++
> libavfilter/version.h | 2 +-
> libavfilter/vf_idet.c | 93
> ++++++++++++++++++++++++++++++++++++++-------------
> libavfilter/vf_idet.h | 7 ++--
> 4 files changed, 80 insertions(+), 27 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 33f842b..ae08f32 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -5625,6 +5625,11 @@ The filter accepts the following options:
> Set interlacing threshold.
> @item prog_thres
> Set progressive threshold.
> +@item half_life
> +Number of frames after which a given frame's contribution to the
> +statistics is halved (i.e., it contributes only 0.5 to it's
> +classification). The default of 0 means that all frames seen are given
> +full weight of 1.0 forever.
> @end table
>
> @section il
> diff --git a/libavfilter/version.h b/libavfilter/version.h
> index 440c587..dab9b45 100644
> --- a/libavfilter/version.h
> +++ b/libavfilter/version.h
> @@ -31,7 +31,7 @@
>
> #define LIBAVFILTER_VERSION_MAJOR 5
> #define LIBAVFILTER_VERSION_MINOR 2
> -#define LIBAVFILTER_VERSION_MICRO 101
> +#define LIBAVFILTER_VERSION_MICRO 102
>
> #define LIBAVFILTER_VERSION_INT AV_VERSION_INT(LIBAVFILTER_VERSION_MAJOR, \
> LIBAVFILTER_VERSION_MINOR, \
> diff --git a/libavfilter/vf_idet.c b/libavfilter/vf_idet.c
> index 6f99f39..464532f 100644
> --- a/libavfilter/vf_idet.c
> +++ b/libavfilter/vf_idet.c
> @@ -32,11 +32,26 @@
> static const AVOption idet_options[] = {
> { "intl_thres", "set interlacing threshold",
> OFFSET(interlace_threshold), AV_OPT_TYPE_FLOAT, {.dbl = 1.04}, -1, FLT_MAX,
> FLAGS },
> { "prog_thres", "set progressive threshold",
> OFFSET(progressive_threshold), AV_OPT_TYPE_FLOAT, {.dbl = 1.5}, -1, FLT_MAX,
> FLAGS },
> + { "half_life", "half life of cumulative statistics", OFFSET(half_life),
> AV_OPT_TYPE_FLOAT, {.dbl = 0.0}, -1, INT_MAX, FLAGS },
> { NULL }
> };
>
> AVFILTER_DEFINE_CLASS(idet);
>
> +static uint64_t uintpow(uint64_t b,unsigned int e)
> +{
> + uint64_t r=1;
> + while(e--) r*=b;
> + return r;
> +}
> +
> +#define PRECISION 1000000000
fix point operations generally work on powers of 2
the code isnt speed critical so it doesnt matter much ATM but it
feels wrong to me
> +#define FLOAT_TO_FXP(value) (uint64_t) round( PRECISION * (value) )
> +#define FXP_TO_INT(value) (value) / PRECISION
> +#define FXP_MULT(a,b) ((a) * (b)) / PRECISION
> +#define FXP_INCREMENT(value) (value) += PRECISION
putting single basic arithmetic operations behind macros is IMO not a
good idea, it makes the code unneccesary hard to read to someone who
doesnt know what the macros do
> +
> static const char *type2str(Type type)
> {
> switch(type) {
> @@ -48,6 +63,14 @@ static const char *type2str(Type type)
> return NULL;
> }
>
> +static int av_dict_set_fxp(AVDictionary **pm, const char *key, uint64_t
> value, unsigned int decimal_digits,
> + int flags)
> +{
> + char valuestr[44];
> + snprintf(valuestr, sizeof(valuestr), "%"PRId64".%0*"PRId64,
> FXP_TO_DECIMAL(value, decimal_digits));
> + return av_dict_set(pm, key, valuestr, flags);
> +}
> +
> int ff_idet_filter_line_c(const uint8_t *a, const uint8_t *b, const uint8_t
> *c, int w)
> {
> int x;
> @@ -74,6 +97,23 @@ int ff_idet_filter_line_c_16bit(const uint16_t *a, const
> uint16_t *b, const uint
> return ret;
> }
>
> +static void log_cumulative_stats(AVFilterContext *ctx)
> +{
> + IDETContext *idet = ctx->priv;
> + av_log(ctx, AV_LOG_INFO, "Single frame detection: TFF:%6"PRId64"
> BFF:%6"PRId64" Progressive:%6"PRId64" Undetermined:%6"PRId64"\n",
> + FXP_TO_INT(idet->prestat[TFF]),
> + FXP_TO_INT(idet->prestat[BFF]),
> + FXP_TO_INT(idet->prestat[PROGRESSIVE]),
> + FXP_TO_INT(idet->prestat[UNDETERMINED])
> + );
> + av_log(ctx, AV_LOG_INFO, "Multi frame detection: TFF:%6"PRId64"
> BFF:%6"PRId64" Progressive:%6"PRId64" Undetermined:%6"PRId64"\n",
> + FXP_TO_INT(idet->poststat[TFF]),
> + FXP_TO_INT(idet->poststat[BFF]),
> + FXP_TO_INT(idet->poststat[PROGRESSIVE]),
> + FXP_TO_INT(idet->poststat[UNDETERMINED])
> + );
this still breaks the final statistics:
actually iam not completely sure why but without specifying half_life
i get only the final statistcis and they look this:
- Single frame detection: TFF:69 BFF:55 Progressive:67 Undetermined:559
- Multi frame detection: TFF:92 BFF:394 Progressive:244 Undetermined:20
+ Single frame detection: TFF: 13 BFF: 18 Progressive: 11
Undetermined: 5
+ Multi frame detection: TFF: 18 BFF: 6 Progressive: 4
Undetermined: 1
before they provided global information about the input
afterwards they dont seem to anymore
with half_life they get printed regularly, but the intent behind the
final statistics was to provide final global statistics and if thats
replaced by exponentioally scaled statics every 50 frames then
thats something else, which may or may not have its use but i
would like to preserve final file global statistics
as they are usefull to awnser the question is the file interlaced
is it progressive, is it both
> +}
> +
> static void filter(AVFilterContext *ctx)
> {
> IDETContext *idet = ctx->priv;
> @@ -146,23 +186,34 @@ static void filter(AVFilterContext *ctx)
> idet->cur->interlaced_frame = 0;
> }
>
> - idet->prestat [ type] ++;
> - idet->poststat[idet->last_type] ++;
> + for(i=0; i<4; i++){
> + idet->prestat [i] = FXP_MULT(idet->decay_coefficient, idet->prestat
> [i]);
> + idet->poststat[i] = FXP_MULT(idet->decay_coefficient,
> idet->poststat[i]);
> + }
> +
> + FXP_INCREMENT(idet->prestat [ type]);
> + FXP_INCREMENT(idet->poststat[idet->last_type]);
>
> av_log(ctx, AV_LOG_DEBUG, "Single frame:%12s, Multi frame:%12s\n",
> type2str(type), type2str(idet->last_type));
>
> - av_dict_set (metadata, "lavfi.idet.single.current_frame",
> type2str(type), 0);
> - av_dict_set_int(metadata, "lavfi.idet.single.tff", idet->prestat[TFF],
> 0);
> - av_dict_set_int(metadata, "lavfi.idet.single.bff", idet->prestat[BFF],
> 0);
> - av_dict_set_int(metadata, "lavfi.idet.single.progressive",
> idet->prestat[PROGRESSIVE], 0);
> - av_dict_set_int(metadata, "lavfi.idet.single.undetermined",
> idet->prestat[UNDETERMINED], 0);
> + idet->frames_since_stats ++;
> + if( idet->half_life > 0.0 &&
> + idet->frames_since_stats > idet->half_life ){
> + log_cumulative_stats(ctx);
> + idet->frames_since_stats = 0;
> + }
>
> - av_dict_set (metadata, "lavfi.idet.multiple.current_frame",
> type2str(idet->last_type), 0);
> - av_dict_set_int(metadata, "lavfi.idet.multiple.tff",
> idet->poststat[TFF], 0);
> - av_dict_set_int(metadata, "lavfi.idet.multiple.bff",
> idet->poststat[BFF], 0);
> - av_dict_set_int(metadata, "lavfi.idet.multiple.progressive",
> idet->poststat[PROGRESSIVE], 0);
> - av_dict_set_int(metadata, "lavfi.idet.multiple.undetermined",
> idet->poststat[UNDETERMINED], 0);
> + av_dict_set (metadata, "lavfi.idet.single.current_frame",
> type2str(type), 0);
> + av_dict_set_fxp(metadata, "lavfi.idet.single.tff",
> idet->prestat[TFF], 2 , 0);
> + av_dict_set_fxp(metadata, "lavfi.idet.single.bff",
> idet->prestat[BFF], 2, 0);
> + av_dict_set_fxp(metadata, "lavfi.idet.single.progressive",
> idet->prestat[PROGRESSIVE], 2, 0);
> + av_dict_set_fxp(metadata, "lavfi.idet.single.undetermined",
> idet->prestat[UNDETERMINED], 2, 0);
>
> + av_dict_set (metadata, "lavfi.idet.multiple.current_frame",
> type2str(idet->last_type), 0);
> + av_dict_set_fxp(metadata, "lavfi.idet.multiple.tff",
> idet->poststat[TFF], 2, 0);
> + av_dict_set_fxp(metadata, "lavfi.idet.multiple.bff",
> idet->poststat[BFF], 2, 0);
> + av_dict_set_fxp(metadata, "lavfi.idet.multiple.progressive",
> idet->poststat[PROGRESSIVE], 2, 0);
> + av_dict_set_fxp(metadata, "lavfi.idet.multiple.undetermined",
> idet->poststat[UNDETERMINED], 2, 0);
> }
>
> static int filter_frame(AVFilterLink *link, AVFrame *picref)
> @@ -228,18 +279,7 @@ static av_cold void uninit(AVFilterContext *ctx)
> {
> IDETContext *idet = ctx->priv;
>
> - av_log(ctx, AV_LOG_INFO, "Single frame detection: TFF:%d BFF:%d
> Progressive:%d Undetermined:%d\n",
> - idet->prestat[TFF],
> - idet->prestat[BFF],
> - idet->prestat[PROGRESSIVE],
> - idet->prestat[UNDETERMINED]
> - );
> - av_log(ctx, AV_LOG_INFO, "Multi frame detection: TFF:%d BFF:%d
> Progressive:%d Undetermined:%d\n",
> - idet->poststat[TFF],
> - idet->poststat[BFF],
> - idet->poststat[PROGRESSIVE],
> - idet->poststat[UNDETERMINED]
> - );
> + log_cumulative_stats(ctx);
factorizing code into a seperate function should ideally be in a
seperate patch, otherwise its harder to see what is changed in the
moved code
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Those who are best at talking, realize last or never when they are wrong.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
