From 2f24290c8edd14262ee8a674a64b1223e1cbbf41 Mon Sep 17 00:00:00 2001
From: Anton Khirnov <anton@khirnov.net>
Date: Tue, 28 Mar 2023 11:48:50 +0200
Subject: fftools/ffmpeg: disable and deprecate -qphist

This option adds a long string of numbers to the progress line, where
i-th number contains the base-2 logarithm of the number of times a frame
with this QP value was seen by print_report().

There are multiple problems with this feature:
* despite this existing since 2005, web search shows no indication
  that it was ever useful for any meaningful purpose;
* the format of what is printed is entirely undocumented, one has to
  find it out from the source code;
* QP values above 31 are silently ignored;
* it only works with one video stream;
* as it relies on global state, it is in conflict with ongoing
  architectural changes.

It then seems that the nontrivial cost of maintaining this option is not
worth its negligible (or possibly negative - since it pollutes the
already large option space) value.
Users who really need similar functionality can also implement it
themselves using -vstats.
---
 fftools/ffmpeg.c     | 10 ----------
 fftools/ffmpeg.h     |  2 +-
 fftools/ffmpeg_opt.c | 15 ++++++++++++---
 3 files changed, 13 insertions(+), 14 deletions(-)

(limited to 'fftools')

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 02a7f20554..74aba28a9b 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -748,7 +748,6 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti
     int64_t pts = INT64_MIN + 1;
     static int64_t last_time = -1;
     static int first_report = 1;
-    static int qp_histogram[52];
     int hours, mins, secs, us;
     const char *hours_sign;
     int ret;
@@ -794,14 +793,6 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti
                        ost->file_index, ost->index, q);
             if (is_last_report)
                 av_bprintf(&buf, "L");
-            if (qp_hist) {
-                int j;
-                int qp = lrintf(q);
-                if (qp >= 0 && qp < FF_ARRAY_ELEMS(qp_histogram))
-                    qp_histogram[qp]++;
-                for (j = 0; j < 32; j++)
-                    av_bprintf(&buf, "%X", av_log2(qp_histogram[j] + 1));
-            }
 
             if (enc && (enc->flags & AV_CODEC_FLAG_PSNR) &&
                 (ost->pict_type != AV_PICTURE_TYPE_NONE || is_last_report)) {
@@ -2321,7 +2312,6 @@ static int check_keyboard_interaction(int64_t cur_time)
     }
     if (key == '+') av_log_set_level(av_log_get_level()+10);
     if (key == '-') av_log_set_level(av_log_get_level()-10);
-    if (key == 's') qp_hist     ^= 1;
     if (key == 'c' || key == 'C'){
         char buf[4096], target[64], command[256], arg[256] = {0};
         double time;
diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h
index bbcbf1aa80..80a1192ed0 100644
--- a/fftools/ffmpeg.h
+++ b/fftools/ffmpeg.h
@@ -54,6 +54,7 @@
 #define FFMPEG_OPT_MAP_CHANNEL 1
 #define FFMPEG_OPT_MAP_SYNC 1
 #define FFMPEG_ROTATION_METADATA 1
+#define FFMPEG_OPT_QPHIST 1
 
 enum VideoSyncMethod {
     VSYNC_AUTO = -1,
@@ -738,7 +739,6 @@ extern int exit_on_error;
 extern int abort_on_flags;
 extern int print_stats;
 extern int64_t stats_period;
-extern int qp_hist;
 extern int stdin_interaction;
 extern AVIOContext *progress_avio;
 extern float max_error_rate;
diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
index 055275d813..aa9aa0e9b4 100644
--- a/fftools/ffmpeg_opt.c
+++ b/fftools/ffmpeg_opt.c
@@ -80,7 +80,6 @@ int debug_ts          = 0;
 int exit_on_error     = 0;
 int abort_on_flags    = 0;
 int print_stats       = -1;
-int qp_hist           = 0;
 int stdin_interaction = 1;
 float max_error_rate  = 2.0/3;
 char *filter_nbthreads;
@@ -1344,6 +1343,14 @@ int opt_timelimit(void *optctx, const char *opt, const char *arg)
     return 0;
 }
 
+#if FFMPEG_OPT_QPHIST
+static int opt_qphist(void *optctx, const char *opt, const char *arg)
+{
+    av_log(NULL, AV_LOG_WARNING, "Option -%s is deprecated and has no effect\n", opt);
+    return 0;
+}
+#endif
+
 #define OFFSET(x) offsetof(OptionsContext, x)
 const OptionDef options[] = {
     /* main options */
@@ -1627,8 +1634,10 @@ const OptionDef options[] = {
     { "vtag",         OPT_VIDEO | HAS_ARG | OPT_EXPERT  | OPT_PERFILE |
                       OPT_INPUT | OPT_OUTPUT,                                    { .func_arg = opt_old2new },
         "force video tag/fourcc", "fourcc/tag" },
-    { "qphist",       OPT_VIDEO | OPT_BOOL | OPT_EXPERT ,                        { &qp_hist },
-        "show QP histogram" },
+#if FFMPEG_OPT_QPHIST
+    { "qphist",       OPT_VIDEO | OPT_EXPERT ,                        { .func_arg = opt_qphist },
+        "deprecated, does nothing" },
+#endif
     { "fps_mode",     OPT_VIDEO | HAS_ARG | OPT_STRING | OPT_EXPERT |
                       OPT_SPEC | OPT_OUTPUT,                                     { .off = OFFSET(fps_mode) },
         "set framerate mode for matching video streams; overrides vsync" },
-- 
cgit v1.2.1