| Message ID | 1472238617-16213-1-git-send-email-bobobo@google.com |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Aug 26, 2016 at 12:10:17PM -0700, Lucas Cooper wrote: > This allows retroactive calculation/aggregation of PSNR from the stats > log. > --- > libavfilter/vf_psnr.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/libavfilter/vf_psnr.c b/libavfilter/vf_psnr.c > index 3bec747..9ad1423 100644 > --- a/libavfilter/vf_psnr.c > +++ b/libavfilter/vf_psnr.c > @@ -45,6 +45,7 @@ typedef struct PSNRContext { > char *stats_file_str; > int stats_version; > int stats_header_written; > + int stats_add_max; > int max[4], average_max; > int is_rgb; > uint8_t rgba_map[4]; > @@ -63,6 +64,7 @@ static const AVOption psnr_options[] = { > {"stats_file", "Set file where to store per-frame difference information", OFFSET(stats_file_str), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 0, FLAGS }, > {"f", "Set file where to store per-frame difference information", OFFSET(stats_file_str), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 0, FLAGS }, > {"stats_version", "Set the format version for the stats file.", OFFSET(stats_version), AV_OPT_TYPE_INT, {.i64=1}, 1, 2, FLAGS }, > + {"output_max", "Add raw stats (max values) to the output log.", OFFSET(stats_add_max), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS}, > { NULL } > }; > > @@ -182,6 +184,12 @@ static AVFrame *do_psnr(AVFilterContext *ctx, AVFrame *main, > for (j = 0; j < s->nb_components; j++) { > fprintf(s->stats_file, ",psnr_%c", s->comps[j]); > } > + if (s->stats_add_max) { > + fprintf(s->stats_file, ",max_avg"); > + for (j = 0; j < s->nb_components; j++) { > + fprintf(s->stats_file, ",max_%c", s->comps[j]); > + } > + } > fprintf(s->stats_file, "\n"); > s->stats_header_written = 1; > } > @@ -196,6 +204,13 @@ static AVFrame *do_psnr(AVFilterContext *ctx, AVFrame *main, > fprintf(s->stats_file, "psnr_%c:%0.2f ", s->comps[j], > get_psnr(comp_mse[c], 1, s->max[c])); > } > + if (s->stats_version == 2 && s->stats_add_max) { > + fprintf(s->stats_file, "max_avg:%d ", s->average_max); > + for (j = 0; j < s->nb_components; j++) { > + c = s->is_rgb ? s->rgba_map[j] : j; > + fprintf(s->stats_file, "max_%c:%d ", s->comps[j], s->max[c]); > + } > + } > fprintf(s->stats_file, "\n"); if the user sets output_max without stats_version 2 this would just silently ignore the option also missing doc/*.texi update [...]
> if the user sets output_max without stats_version 2 this would just silently ignore the option Do you think this warrants a fatal error? A warning seems insufficient as the output_max option is explicitly requested by the user. > also missing doc/*.texi update Should I send that in a followup patch once this one is finalized or would you prefer them both in the same patch? On 27 August 2016 at 03:13, Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Aug 26, 2016 at 12:10:17PM -0700, Lucas Cooper wrote: > > This allows retroactive calculation/aggregation of PSNR from the stats > > log. > > --- > > libavfilter/vf_psnr.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/libavfilter/vf_psnr.c b/libavfilter/vf_psnr.c > > index 3bec747..9ad1423 100644 > > --- a/libavfilter/vf_psnr.c > > +++ b/libavfilter/vf_psnr.c > > @@ -45,6 +45,7 @@ typedef struct PSNRContext { > > char *stats_file_str; > > int stats_version; > > int stats_header_written; > > + int stats_add_max; > > int max[4], average_max; > > int is_rgb; > > uint8_t rgba_map[4]; > > @@ -63,6 +64,7 @@ static const AVOption psnr_options[] = { > > {"stats_file", "Set file where to store per-frame difference > information", OFFSET(stats_file_str), AV_OPT_TYPE_STRING, {.str=NULL}, 0, > 0, FLAGS }, > > {"f", "Set file where to store per-frame difference > information", OFFSET(stats_file_str), AV_OPT_TYPE_STRING, {.str=NULL}, 0, > 0, FLAGS }, > > {"stats_version", "Set the format version for the stats file.", > OFFSET(stats_version), AV_OPT_TYPE_INT, {.i64=1}, 1, 2, > FLAGS }, > > + {"output_max", "Add raw stats (max values) to the output log.", > OFFSET(stats_add_max), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS}, > > { NULL } > > }; > > > > @@ -182,6 +184,12 @@ static AVFrame *do_psnr(AVFilterContext *ctx, > AVFrame *main, > > for (j = 0; j < s->nb_components; j++) { > > fprintf(s->stats_file, ",psnr_%c", s->comps[j]); > > } > > + if (s->stats_add_max) { > > + fprintf(s->stats_file, ",max_avg"); > > + for (j = 0; j < s->nb_components; j++) { > > + fprintf(s->stats_file, ",max_%c", s->comps[j]); > > + } > > + } > > fprintf(s->stats_file, "\n"); > > s->stats_header_written = 1; > > } > > @@ -196,6 +204,13 @@ static AVFrame *do_psnr(AVFilterContext *ctx, > AVFrame *main, > > fprintf(s->stats_file, "psnr_%c:%0.2f ", s->comps[j], > > get_psnr(comp_mse[c], 1, s->max[c])); > > } > > + if (s->stats_version == 2 && s->stats_add_max) { > > + fprintf(s->stats_file, "max_avg:%d ", s->average_max); > > + for (j = 0; j < s->nb_components; j++) { > > + c = s->is_rgb ? s->rgba_map[j] : j; > > + fprintf(s->stats_file, "max_%c:%d ", s->comps[j], > s->max[c]); > > + } > > + } > > fprintf(s->stats_file, "\n"); > > if the user sets output_max without stats_version 2 this would just > silently ignore the option > also missing doc/*.texi update > > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > Awnsering whenever a program halts or runs forever is > On a turing machine, in general impossible (turings halting problem). > On any real computer, always possible as a real computer has a finite > number > of states N, and will either halt in less than N cycles or never halt. > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > >
On Mon, Aug 29, 2016 at 01:12:10PM -0700, Lucas Cooper wrote: > > if the user sets output_max without stats_version 2 this would just > silently ignore the option > > Do you think this warrants a fatal error? A warning seems insufficient as > the output_max option is explicitly requested by the user. yes > > > also missing doc/*.texi update > > Should I send that in a followup patch once this one is finalized or would > you prefer them both in the same patch? iam fine with either but a single patch for docs and corresponding feature makes more sense to me thx [...]
diff --git a/libavfilter/vf_psnr.c b/libavfilter/vf_psnr.c index 3bec747..9ad1423 100644 --- a/libavfilter/vf_psnr.c +++ b/libavfilter/vf_psnr.c @@ -45,6 +45,7 @@ typedef struct PSNRContext { char *stats_file_str; int stats_version; int stats_header_written; + int stats_add_max; int max[4], average_max; int is_rgb; uint8_t rgba_map[4]; @@ -63,6 +64,7 @@ static const AVOption psnr_options[] = { {"stats_file", "Set file where to store per-frame difference information", OFFSET(stats_file_str), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 0, FLAGS }, {"f", "Set file where to store per-frame difference information", OFFSET(stats_file_str), AV_OPT_TYPE_STRING, {.str=NULL}, 0, 0, FLAGS }, {"stats_version", "Set the format version for the stats file.", OFFSET(stats_version), AV_OPT_TYPE_INT, {.i64=1}, 1, 2, FLAGS }, + {"output_max", "Add raw stats (max values) to the output log.", OFFSET(stats_add_max), AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS}, { NULL } }; @@ -182,6 +184,12 @@ static AVFrame *do_psnr(AVFilterContext *ctx, AVFrame *main, for (j = 0; j < s->nb_components; j++) { fprintf(s->stats_file, ",psnr_%c", s->comps[j]); } + if (s->stats_add_max) { + fprintf(s->stats_file, ",max_avg"); + for (j = 0; j < s->nb_components; j++) { + fprintf(s->stats_file, ",max_%c", s->comps[j]); + } + } fprintf(s->stats_file, "\n"); s->stats_header_written = 1; } @@ -196,6 +204,13 @@ static AVFrame *do_psnr(AVFilterContext *ctx, AVFrame *main, fprintf(s->stats_file, "psnr_%c:%0.2f ", s->comps[j], get_psnr(comp_mse[c], 1, s->max[c])); } + if (s->stats_version == 2 && s->stats_add_max) { + fprintf(s->stats_file, "max_avg:%d ", s->average_max); + for (j = 0; j < s->nb_components; j++) { + c = s->is_rgb ? s->rgba_map[j] : j; + fprintf(s->stats_file, "max_%c:%d ", s->comps[j], s->max[c]); + } + } fprintf(s->stats_file, "\n"); }