Only call hdcd_update_info() when the control code changes
instead of every frame, so the counters are more meaningful.

Also, adds some basic error detection. After scanning through
about 30 HDCD-encoded CD's I've found errors where a code
is signaled, but then fails to match one of the patterns, so
it is ignored. Often, it is very close to a code, differing
by only one bit. I don't know enough to say whether it is
using some unknown feature of HDCD that this filter doesn't
know about, or if there was just an error in the encoding.
I've added some comments so that it should be easier for
others to examine.

An example with many errors is Līve - Secret Samadhi, if
anyone wants to look into it.

Patch is attached.
Any comments would be appreciated.
From 63a2736dd8444fe64d8c5bc885d61979bfe93f6b Mon Sep 17 00:00:00 2001
From: Burt P <pbu...@gmail.com>
Date: Mon, 11 Jul 2016 17:08:20 -0500
Subject: [PATCH] af_hdcd.c: only hdcd_update_info() when something changes,
 add error detection
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* hdcd_update_info() when control code changes; counts are more meaningful
* some encoding errors can be detected
* fewer false positives by ignoring code_counterC in HDCD detection
* integrate() renamed hdcd_integrate() to be consistent with the other function names
* some code comments

Try LÄ«ve - Secret Samadhi or John Mellencamp - Mr. Happy Go Lucky for error detection.

Signed-off-by: Burt P <pbu...@gmail.com>
---
 libavfilter/af_hdcd.c | 122 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 79 insertions(+), 43 deletions(-)

diff --git a/libavfilter/af_hdcd.c b/libavfilter/af_hdcd.c
index 89012eb..72267d5 100644
--- a/libavfilter/af_hdcd.c
+++ b/libavfilter/af_hdcd.c
@@ -822,8 +822,11 @@ typedef struct {
     int running_gain;
     unsigned sustain, sustain_reset;
     int code_counterA;
+    int code_counterA_almost; /* looks like an A code, but a bit expected to be 0 is 1 */
     int code_counterB;
+    int code_counterB_checkfails; /* looks like a B code, but doesn't pass the XOR check */
     int code_counterC;
+    int code_counterC_unmatched; /* told to look for a code, but didn't find one */
 
     /* For user information/stats, pulled up into HDCDContext
      * by filter_frame() */
@@ -835,7 +838,6 @@ typedef struct {
      * steps of 0.5, but no value below -6.0 dB should appear. */
     int gain_counts[16]; /* for cursiosity, mostly */
     int max_gain;
-    int cb6, cb7; /* watch bits 6 and 7 of the control code, for curiosity */
 } hdcd_state_t;
 
 typedef struct HDCDContext {
@@ -844,6 +846,7 @@ typedef struct HDCDContext {
 
     /* User information/stats */
     int hdcd_detected;
+    int det_errors;            /* detectable errors */
     int uses_peak_extend;
     int uses_transient_filter; /* detected, but not implemented */
     float max_gain_adjustment; /* in dB, expected in the range -6.0 to 0.0 */
@@ -872,18 +875,28 @@ static void hdcd_reset(hdcd_state_t *state, unsigned rate)
     state->sustain_reset = rate * 10;
 
     state->code_counterA = 0;
+    state->code_counterA_almost = 0;
     state->code_counterB = 0;
+    state->code_counterB_checkfails = 0;
     state->code_counterC = 0;
+    state->code_counterC_unmatched = 0;
 
     state->count_peak_extend = 0;
     state->count_transient_filter = 0;
     for(i = 0; i < 16; i++) state->gain_counts[i] = 0;
     state->max_gain = 0;
-    state->cb6 = 0;
-    state->cb7 = 0;
 }
 
-static int integrate(hdcd_state_t *state, int *flag, const int32_t *samples, int count, int stride)
+/* Update the user info/counters */
+static void hdcd_update_info(hdcd_state_t *state)
+{
+    if (state->control & 16) state->count_peak_extend++;
+    if (state->control & 32) state->count_transient_filter++;
+    state->gain_counts[state->control & 15]++;
+    state->max_gain = FFMAX(state->max_gain, (state->control & 15));
+}
+
+static int hdcd_integrate(hdcd_state_t *state, int *flag, const int32_t *samples, int count, int stride)
 {
     uint32_t bits = 0;
     int result = FFMIN(state->readahead, count);
@@ -903,19 +916,45 @@ static int integrate(hdcd_state_t *state, int *flag, const int32_t *samples, int
     bits = (state->window ^ state->window >> 5 ^ state->window >> 23);
 
     if (state->arg) {
-        if ((bits & 0xffffffc8) == 0x0fa00500) {
-            state->control = (bits & 255) + (bits & 7);
-            *flag = 1;
-            state->code_counterA++;
-        }
-        if (((bits ^ (~bits >> 8 & 255)) & 0xffff00ff) == 0xa0060000) {
-            state->control = bits >> 8 & 255;
-            *flag = 1;
-            state->code_counterB++;
+        if ((bits & 0x0fa00500) == 0x0fa00500) {
+            /* A: 8-bit code */
+            if ((bits & 0xc8) == 0) {
+                /*                   [..pt gggg]
+                 * 0x0fa005[..] -> 0b[00.. 0...], gain part doubled */
+                state->control = (bits & 255) + (bits & 7);
+                *flag = 1;
+                state->code_counterA++;
+            } else {
+                /* one of bits 3, 6, or 7 was not 0 */
+                state->code_counterA_almost++;
+                av_log(0, AV_LOG_VERBOSE,
+                    "hdcd error: Control A almost: 0x%08x\n", bits);
+            }
+        } else if ((bits & 0xa0060000) == 0xa0060000) {
+            /* B: 8-bit code, 8-bit XOR check */
+            if (((bits ^ (~bits >> 8 & 255)) & 0xffff00ff) == 0xa0060000) {
+                /*          check:   [..pt gggg ~(..pt gggg)]
+                 * 0xa006[....] -> 0b[.... ....   .... .... ] */
+                state->control = bits >> 8 & 255;
+                *flag = 1;
+                state->code_counterB++;
+            } else {
+                /* XOR check failed */
+                state->code_counterB_checkfails++;
+                av_log(0, AV_LOG_VERBOSE,
+                    "hdcd error: Control B check failed: 0x%08x\n", bits);
+            }
+        } else {
+            /* told to look for a code, but didn't match one */
+            state->code_counterC_unmatched++;
+            av_log(0, AV_LOG_VERBOSE,
+                "hdcd error: Unmatched code: 0x%08x\n", bits);
         }
+        if (*flag) hdcd_update_info(state);
         state->arg = 0;
     }
     if (bits == 0x7e0fa005 || bits == 0x7e0fa006) {
+        /* Expecting an A (8 bits), or B (16 bits) code */
         state->readahead = (bits & 3) * 8;
         state->arg = 1;
         state->code_counterC++;
@@ -941,7 +980,7 @@ static int hdcd_scan(hdcd_state_t *state, const int32_t *samples, int max, int s
     result = 0;
     while (result < max) {
         int flag;
-        int consumed = integrate(state, &flag, samples, max - result, stride);
+        int consumed = hdcd_integrate(state, &flag, samples, max - result, stride);
         result += consumed;
         if (flag > 0) {
             state->sustain = state->sustain_reset;
@@ -952,6 +991,7 @@ static int hdcd_scan(hdcd_state_t *state, const int32_t *samples, int max, int s
     return result;
 }
 
+/* Apply HDCD peak extend and low level gain adjustment */
 static int hdcd_envelope(int32_t *samples, int count, int stride, int gain, int target_gain, int extend)
 {
     int i;
@@ -1011,18 +1051,6 @@ static int hdcd_envelope(int32_t *samples, int count, int stride, int gain, int
     return gain;
 }
 
-/* update the user info/flags */
-static void hdcd_update_info(hdcd_state_t *state)
-{
-    if (state->control & 16) state->count_peak_extend++;
-    if (state->control & 32) state->count_transient_filter++;
-    state->gain_counts[state->control & 15]++;
-    state->max_gain = FFMAX(state->max_gain, (state->control & 15));
-
-    if (state->control & 64) state->cb6++;
-    if (state->control & 128) state->cb7++;
-}
-
 static void hdcd_process(hdcd_state_t *state, int32_t *samples, int count, int stride)
 {
     int32_t *samples_end = samples + count * stride;
@@ -1031,8 +1059,6 @@ static void hdcd_process(hdcd_state_t *state, int32_t *samples, int count, int s
     int target_gain = (state->control & 15) << 7;
     int lead = 0;
 
-    hdcd_update_info(state);
-
     while (count > lead) {
         int envelope_run;
         int run;
@@ -1049,7 +1075,6 @@ static void hdcd_process(hdcd_state_t *state, int32_t *samples, int count, int s
         lead = run - envelope_run;
         peak_extend = (state->control & 16);
         target_gain = (state->control & 15) << 7;
-        hdcd_update_info(state);
     }
     if (lead > 0) {
         av_assert0(samples + lead * stride <= samples_end);
@@ -1061,7 +1086,7 @@ static void hdcd_process(hdcd_state_t *state, int32_t *samples, int count, int s
 
 /* convert to float from 4-bit (3.1) fixed-point
  * the always-negative value is stored positive, so make it negative */
-#define GAINTOFLOAT(g) (g) ? -(float)(g>>1) - ((g & 1) ? 0.5 : 0.0) : 0.0
+#define GAINTOFLOAT(g) (g ? -(float)(g>>1) - ((g & 1) ? 0.5 : 0.0) : 0.0)
 
 static int filter_frame(AVFilterLink *inlink, AVFrame *in)
 {
@@ -1087,6 +1112,7 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
         out_data[n] = in_data[n];
     }
 
+    s->det_errors = 0;
     for (c = 0; c < inlink->channels; c++) {
         hdcd_state_t *state = &s->state[c];
         hdcd_process(state, out_data + c, in->nb_samples, out->channels);
@@ -1094,7 +1120,10 @@ static int filter_frame(AVFilterLink *inlink, AVFrame *in)
         s->uses_peak_extend |= !!state->count_peak_extend;
         s->uses_transient_filter |= !!state->count_transient_filter;
         s->max_gain_adjustment = FFMIN(s->max_gain_adjustment, GAINTOFLOAT(state->max_gain));
-        s->hdcd_detected |= state->code_counterC || state->code_counterB || state->code_counterA;
+        s->det_errors += state->code_counterA_almost
+            + state->code_counterB_checkfails
+            + state->code_counterC_unmatched;
+        s->hdcd_detected |= state->code_counterB || state->code_counterA;
     }
 
     av_frame_free(&in);
@@ -1155,23 +1184,30 @@ static av_cold void uninit(AVFilterContext *ctx)
     /* dump the state for each channel for AV_LOG_VERBOSE */
     for (i = 0; i < 2; i++) {
         hdcd_state_t *state = &s->state[i];
-        av_log(ctx, AV_LOG_VERBOSE, "Channel %d: counter A: %d, B: %d, C: %d\n", i, state->code_counterA,
-                state->code_counterB, state->code_counterC);
-        av_log(ctx, AV_LOG_VERBOSE, "Channel %d: c(pe): %d, c(tf): %d, cb6: %d, cb7: %d\n", i,
-                state->count_peak_extend, state->count_transient_filter, state->cb6, state->cb7);
+        av_log(ctx, AV_LOG_VERBOSE, "Channel %d: counter A: %d, B: %d, C: %d\n", i,
+                state->code_counterA, state->code_counterB, state->code_counterC);
+        av_log(ctx, AV_LOG_VERBOSE, "Channel %d: pe: %d, tf: %d, almost_A: %d, checkfail_B: %d, unmatched_C: %d\n", i,
+            state->count_peak_extend,
+            state->count_transient_filter,
+            state->code_counterA_almost,
+            state->code_counterB_checkfails,
+            state->code_counterC_unmatched);
         for (j = 0; j <= state->max_gain; j++) {
-            av_log(ctx, AV_LOG_VERBOSE, "Channel %d: tg %0.1f - %d\n", i, GAINTOFLOAT(j), state->gain_counts[j]);
+            av_log(ctx, AV_LOG_VERBOSE, "Channel %d: tg %0.1f: %d\n", i, GAINTOFLOAT(j), state->gain_counts[j]);
         }
     }
 
     /* log the HDCD decode information */
-    av_log(ctx, AV_LOG_INFO,
-        "HDCD detected: %s, peak_extend: %s, max_gain_adj: %0.1f dB, transient_filter: %s\n",
-        (s->hdcd_detected) ? "yes" : "no",
-        (s->uses_peak_extend) ? "enabled" : "never enabled",
-        s->max_gain_adjustment,
-        (s->uses_transient_filter) ? "detected" : "not detected"
-        );
+    if (s->hdcd_detected)
+        av_log(ctx, AV_LOG_INFO,
+            "HDCD detected: yes, peak_extend: %s, max_gain_adj: %0.1f dB, transient_filter: %s, detectable errors: %d%s\n",
+            (s->uses_peak_extend) ? "enabled" : "never enabled",
+            s->max_gain_adjustment,
+            (s->uses_transient_filter) ? "detected" : "not detected",
+            s->det_errors, (s->det_errors) ? " (try -v verbose)" : ""
+            );
+    else
+        av_log(ctx, AV_LOG_INFO, "HDCD detected: no\n");
 }
 
 static av_cold int init(AVFilterContext *ctx)
-- 
2.7.4

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Reply via email to