opus_pvq: fix PVQ search for K < 5 and low Ns
authorRostislav Pehlivanov <atomnuker@gmail.com>
Sun, 19 Feb 2017 18:20:39 +0000 (18:20 +0000)
committerRostislav Pehlivanov <atomnuker@gmail.com>
Sun, 19 Feb 2017 19:13:14 +0000 (19:13 +0000)
If the PVQ search picked a place to increment/decrement on the y[]
vector which had no pulse then it would cause a desync since it would
change the sum in the wrong direction. Fix this by not considering
places without pulses as viable.

This makes the PVQ search slightly worse at K < 5 which isn't all that
common. Still, this is a workaround to prevent making broken files until
I can think of a better way of fixing it.

Also add an assertion, which can be removed or moved to assert1/2 once
the PVQ search is stable.

Signed-off-by: Rostislav Pehlivanov <atomnuker@gmail.com>
libavcodec/opus_pvq.c

index 4197ccd..5920ab0 100644 (file)
@@ -277,7 +277,7 @@ static inline int celt_compute_qn(int N, int b, int offset, int pulse_cap,
 }
 
 /* Convert the quantized vector to an index */
-static inline uint32_t celt_icwrsi(uint32_t N, const int *y)
+static inline uint32_t celt_icwrsi(uint32_t N, uint32_t K, const int *y)
 {
     int i, idx = 0, sum = 0;
     for (i = N - 1; i >= 0; i--) {
@@ -285,6 +285,7 @@ static inline uint32_t celt_icwrsi(uint32_t N, const int *y)
         idx += CELT_PVQ_U(N - i, sum) + (y[i] < 0)*i_s;
         sum += FFABS(y[i]);
     }
+    av_assert0(sum == K);
     return idx;
 }
 
@@ -376,7 +377,7 @@ static inline uint64_t celt_cwrsi(uint32_t N, uint32_t K, uint32_t i, int *y)
 
 static inline void celt_encode_pulses(OpusRangeCoder *rc, int *y, uint32_t N, uint32_t K)
 {
-    ff_opus_rc_enc_uint(rc, celt_icwrsi(N, y), CELT_PVQ_V(N, K));
+    ff_opus_rc_enc_uint(rc, celt_icwrsi(N, K, y), CELT_PVQ_V(N, K));
 }
 
 static inline float celt_decode_pulses(OpusRangeCoder *rc, int *y, uint32_t N, uint32_t K)
@@ -415,7 +416,8 @@ static void celt_pvq_search(float *X, int *y, int K, int N)
             float xy_new = xy_norm + 1*phase*FFABS(X[i]);
             float y_new  = y_norm  + 2*phase*FFABS(y[i]);
             xy_new = xy_new * xy_new;
-            if ((max_den*xy_new) > (y_new*max_num)) {
+            /* FIXME: the y[i] check makes the search slightly worse at Ks below 5 */
+            if (y[i] && (max_den*xy_new) > (y_new*max_num)) {
                 max_den = y_new;
                 max_num = xy_new;
                 max_idx = i;