Re: [FFmpeg-devel] [PATCH] avfilter/af_astats: count number of NaNs/Infs/denormals for floating-point audio too
On 2019-04-25 18:49 +0200, Paul B Mahol wrote: > On 4/23/19, Alexander Strasser wrote: [...] > > > > On 2019-04-22 11:51 +0200, Paul B Mahol wrote: > >> Signed-off-by: Paul B Mahol > >> --- > >> doc/filters.texi| 6 +++ > >> libavfilter/af_astats.c | 86 ++--- > >> 2 files changed, 86 insertions(+), 6 deletions(-) > > > > I was a bit surprised when I first saw the number of lines it > > takes to add this feature. OTOH this is not a problem of this > > patch, because it is mostly caused by the structure of the > > code that was in place before. > > > > Changing the structure doesn't seem worth it yet. If > > there will be an addition that is applicable to all the > > individual stats, it should IMHO be reconsidered. > > What are you proposing to change? Sorry if I wasn't clear enough, but as I stated above I am not proposing to change things now. No objections against your patch! To clarify a bit what I meant with changing structure: There seem to be many places where things are duplicated for each individual statistic. Maybe things could be re-organized, so that it isn't needed to repeat those for every of the 2 dozen statistics that can be measured. But that's easier said than done, and even if one tried to do it, one could end up after a lot of work with a result that's not good enough :( That's why I said, maybe to reconsider it next time, when something gets changed that touches all statistics (if that will ever happen), because one would have to touch everything anyway and would probably be in good position (detailed mental model of the entire code of that filter) to try out structural changes. Pardon me, I'm having problems to express this more precisely at the moment. I hope the message got through. Alexander ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/af_astats: count number of NaNs/Infs/denormals for floating-point audio too
On 4/23/19, Alexander Strasser wrote: > Hi Paul, > > just three small comments from me... > > On 2019-04-22 11:51 +0200, Paul B Mahol wrote: >> Signed-off-by: Paul B Mahol >> --- >> doc/filters.texi| 6 +++ >> libavfilter/af_astats.c | 86 ++--- >> 2 files changed, 86 insertions(+), 6 deletions(-) > > I was a bit surprised when I first saw the number of lines it > takes to add this feature. OTOH this is not a problem of this > patch, because it is mostly caused by the structure of the > code that was in place before. > > Changing the structure doesn't seem worth it yet. If > there will be an addition that is applicable to all the > individual stats, it should IMHO be reconsidered. What are you proposing to change? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] avfilter/af_astats: count number of NaNs/Infs/denormals for floating-point audio too
Hi Paul, just three small comments from me... On 2019-04-22 11:51 +0200, Paul B Mahol wrote: > Signed-off-by: Paul B Mahol > --- > doc/filters.texi| 6 +++ > libavfilter/af_astats.c | 86 ++--- > 2 files changed, 86 insertions(+), 6 deletions(-) I was a bit surprised when I first saw the number of lines it takes to add this feature. OTOH this is not a problem of this patch, because it is mostly caused by the structure of the code that was in place before. Changing the structure doesn't seem worth it yet. If there will be an addition that is applicable to all the individual stats, it should IMHO be reconsidered. > diff --git a/doc/filters.texi b/doc/filters.texi > index cfff9b1f4d..945c557e8f 100644 > --- a/doc/filters.texi > +++ b/doc/filters.texi > @@ -2141,6 +2141,9 @@ Bit_depth > Dynamic_range > Zero_crossings > Zero_crossings_rate > +Number_of_NaNs > +Number_of_Infs > +Number_of_denormals Nit: Maybe Number_of_NaNs and Number_of_Infs should not be capitalized like that, e.g. just use Number_of_nans and Number_of_infs . Anyway if there's a reason for choosing this names, it should be OK as it is. Just thinking it is harder to type these from memory if the spelling is a bit arbitrary. > and for Overall: > DC_offset > @@ -2158,6 +2161,9 @@ Flat_factor > Peak_count > Bit_depth > Number_of_samples > +Number_of_NaNs > +Number_of_Infs > +Number_of_denormals > > For example full key look like this @code{lavfi.astats.1.DC_offset} or > this @code{lavfi.astats.Overall.Peak_count}. > diff --git a/libavfilter/af_astats.c b/libavfilter/af_astats.c > index 92368793c2..25c700b344 100644 > --- a/libavfilter/af_astats.c > +++ b/libavfilter/af_astats.c > @@ -20,6 +20,7 @@ > */ > > #include > +#include > > #include "libavutil/opt.h" > #include "audio.h" > @@ -48,6 +49,9 @@ > #define MEASURE_ZERO_CROSSINGS (1 << 16) > #define MEASURE_ZERO_CROSSINGS_RATE (1 << 17) > #define MEASURE_NUMBER_OF_SAMPLES (1 << 18) > +#define MEASURE_NUMBER_OF_NANS (1 << 19) > +#define MEASURE_NUMBER_OF_INFS (1 << 20) > +#define MEASURE_NUMBER_OF_DENORMALS (1 << 21) > > #define MEASURE_MINMAXPEAK (MEASURE_MIN_LEVEL | > MEASURE_MAX_LEVEL | MEASURE_PEAK_LEVEL) > > @@ -68,6 +72,9 @@ typedef struct ChannelStats { > uint64_t min_count, max_count; > uint64_t zero_runs; > uint64_t nb_samples; > +uint64_t nb_nans; > +uint64_t nb_infs; > +uint64_t nb_denormals; > } ChannelStats; > > typedef struct AudioStatsContext { > @@ -83,6 +90,8 @@ typedef struct AudioStatsContext { > int maxbitdepth; > int measure_perchannel; > int measure_overall; > +int is_float; > +int is_double; > } AudioStatsContext; > > #define OFFSET(x) offsetof(AudioStatsContext, x) > @@ -114,6 +123,9 @@ static const AVOption astats_options[] = { >{ "Zero_crossings", "", 0, AV_OPT_TYPE_CONST, > {.i64=MEASURE_ZERO_CROSSINGS }, 0, 0, FLAGS, "measure" }, >{ "Zero_crossings_rate" , "", 0, AV_OPT_TYPE_CONST, > {.i64=MEASURE_ZERO_CROSSINGS_RATE }, 0, 0, FLAGS, "measure" }, >{ "Number_of_samples" , "", 0, AV_OPT_TYPE_CONST, > {.i64=MEASURE_NUMBER_OF_SAMPLES }, 0, 0, FLAGS, "measure" }, > + { "Number_of_NaNs", "", 0, AV_OPT_TYPE_CONST, > {.i64=MEASURE_NUMBER_OF_NANS }, 0, 0, FLAGS, "measure" }, > + { "Number_of_Infs", "", 0, AV_OPT_TYPE_CONST, > {.i64=MEASURE_NUMBER_OF_INFS }, 0, 0, FLAGS, "measure" }, > + { "Number_of_denormals" , "", 0, AV_OPT_TYPE_CONST, > {.i64=MEASURE_NUMBER_OF_DENORMALS }, 0, 0, FLAGS, "measure" }, > { "measure_overall", "only measure_perchannel these overall statistics", > OFFSET(measure_overall), AV_OPT_TYPE_FLAGS, {.i64=MEASURE_ALL}, 0, UINT_MAX, > FLAGS, "measure" }, > { NULL } > }; > @@ -181,6 +193,9 @@ static void reset_stats(AudioStatsContext *s) > p->max_count = 0; > p->zero_runs = 0; > p->nb_samples = 0; > +p->nb_nans = 0; > +p->nb_infs = 0; > +p->nb_denormals = 0; > } > } > > @@ -196,6 +211,11 @@ static int config_output(AVFilterLink *outlink) > s->tc_samples = 5 * s->time_constant * outlink->sample_rate + .5; > s->nb_frames = 0; > s->maxbitdepth = av_get_bytes_per_sample(outlink->format) * 8; > +s->is_double = outlink->format == AV_SAMPLE_FMT_DBL || > + outlink->format == AV_SAMPLE_FMT_DBLP; > + > +s->is_float = outlink->format == AV_SAMPLE_FMT_FLT || > + outlink->format == AV_SAMPLE_FMT_FLTP; > > reset_stats(s); > > @@ -280,6 +300,24 @@ static inline void update_stat(AudioStatsContext *s, > ChannelStats *p, double d, > p->nb_samples++; > } > > +static inline void update_float_stat(AudioStatsContext *s, ChannelStats *p, > float d) I guess "float d" is rather uncommon, but it's not unseen in the FFmpeg code base. > +{ > +
[FFmpeg-devel] [PATCH] avfilter/af_astats: count number of NaNs/Infs/denormals for floating-point audio too
Signed-off-by: Paul B Mahol --- doc/filters.texi| 6 +++ libavfilter/af_astats.c | 86 ++--- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/doc/filters.texi b/doc/filters.texi index cfff9b1f4d..945c557e8f 100644 --- a/doc/filters.texi +++ b/doc/filters.texi @@ -2141,6 +2141,9 @@ Bit_depth Dynamic_range Zero_crossings Zero_crossings_rate +Number_of_NaNs +Number_of_Infs +Number_of_denormals and for Overall: DC_offset @@ -2158,6 +2161,9 @@ Flat_factor Peak_count Bit_depth Number_of_samples +Number_of_NaNs +Number_of_Infs +Number_of_denormals For example full key look like this @code{lavfi.astats.1.DC_offset} or this @code{lavfi.astats.Overall.Peak_count}. diff --git a/libavfilter/af_astats.c b/libavfilter/af_astats.c index 92368793c2..25c700b344 100644 --- a/libavfilter/af_astats.c +++ b/libavfilter/af_astats.c @@ -20,6 +20,7 @@ */ #include +#include #include "libavutil/opt.h" #include "audio.h" @@ -48,6 +49,9 @@ #define MEASURE_ZERO_CROSSINGS (1 << 16) #define MEASURE_ZERO_CROSSINGS_RATE (1 << 17) #define MEASURE_NUMBER_OF_SAMPLES (1 << 18) +#define MEASURE_NUMBER_OF_NANS (1 << 19) +#define MEASURE_NUMBER_OF_INFS (1 << 20) +#define MEASURE_NUMBER_OF_DENORMALS (1 << 21) #define MEASURE_MINMAXPEAK (MEASURE_MIN_LEVEL | MEASURE_MAX_LEVEL | MEASURE_PEAK_LEVEL) @@ -68,6 +72,9 @@ typedef struct ChannelStats { uint64_t min_count, max_count; uint64_t zero_runs; uint64_t nb_samples; +uint64_t nb_nans; +uint64_t nb_infs; +uint64_t nb_denormals; } ChannelStats; typedef struct AudioStatsContext { @@ -83,6 +90,8 @@ typedef struct AudioStatsContext { int maxbitdepth; int measure_perchannel; int measure_overall; +int is_float; +int is_double; } AudioStatsContext; #define OFFSET(x) offsetof(AudioStatsContext, x) @@ -114,6 +123,9 @@ static const AVOption astats_options[] = { { "Zero_crossings", "", 0, AV_OPT_TYPE_CONST, {.i64=MEASURE_ZERO_CROSSINGS }, 0, 0, FLAGS, "measure" }, { "Zero_crossings_rate" , "", 0, AV_OPT_TYPE_CONST, {.i64=MEASURE_ZERO_CROSSINGS_RATE }, 0, 0, FLAGS, "measure" }, { "Number_of_samples" , "", 0, AV_OPT_TYPE_CONST, {.i64=MEASURE_NUMBER_OF_SAMPLES }, 0, 0, FLAGS, "measure" }, + { "Number_of_NaNs", "", 0, AV_OPT_TYPE_CONST, {.i64=MEASURE_NUMBER_OF_NANS }, 0, 0, FLAGS, "measure" }, + { "Number_of_Infs", "", 0, AV_OPT_TYPE_CONST, {.i64=MEASURE_NUMBER_OF_INFS }, 0, 0, FLAGS, "measure" }, + { "Number_of_denormals" , "", 0, AV_OPT_TYPE_CONST, {.i64=MEASURE_NUMBER_OF_DENORMALS }, 0, 0, FLAGS, "measure" }, { "measure_overall", "only measure_perchannel these overall statistics", OFFSET(measure_overall), AV_OPT_TYPE_FLAGS, {.i64=MEASURE_ALL}, 0, UINT_MAX, FLAGS, "measure" }, { NULL } }; @@ -181,6 +193,9 @@ static void reset_stats(AudioStatsContext *s) p->max_count = 0; p->zero_runs = 0; p->nb_samples = 0; +p->nb_nans = 0; +p->nb_infs = 0; +p->nb_denormals = 0; } } @@ -196,6 +211,11 @@ static int config_output(AVFilterLink *outlink) s->tc_samples = 5 * s->time_constant * outlink->sample_rate + .5; s->nb_frames = 0; s->maxbitdepth = av_get_bytes_per_sample(outlink->format) * 8; +s->is_double = outlink->format == AV_SAMPLE_FMT_DBL || + outlink->format == AV_SAMPLE_FMT_DBLP; + +s->is_float = outlink->format == AV_SAMPLE_FMT_FLT || + outlink->format == AV_SAMPLE_FMT_FLTP; reset_stats(s); @@ -280,6 +300,24 @@ static inline void update_stat(AudioStatsContext *s, ChannelStats *p, double d, p->nb_samples++; } +static inline void update_float_stat(AudioStatsContext *s, ChannelStats *p, float d) +{ +int type = fpclassify(d); + +p->nb_nans += type == FP_NAN; +p->nb_infs += type == FP_INFINITE; +p->nb_denormals += type == FP_SUBNORMAL; +} + +static inline void update_double_stat(AudioStatsContext *s, ChannelStats *p, double d) +{ +int type = fpclassify(d); + +p->nb_nans += type == FP_NAN; +p->nb_infs += type == FP_INFINITE; +p->nb_denormals += type == FP_SUBNORMAL; +} + static void set_meta(AVDictionary **metadata, int chan, const char *key, const char *fmt, double val) { @@ -299,6 +337,7 @@ static void set_meta(AVDictionary **metadata, int chan, const char *key, static void set_metadata(AudioStatsContext *s, AVDictionary **metadata) { uint64_t mask = 0, imask = 0x, min_count = 0, max_count = 0, nb_samples = 0; +uint64_t nb_nans = 0, nb_infs = 0, nb_denormals = 0; double min_runs = 0, max_runs = 0, min = DBL_MAX, max = DBL_MIN, min_diff = DBL_MAX, max_diff = 0, nmin = DBL_MAX, nmax = DBL_MIN, @@ -337,6 +376,9 @@ static void