Reduce valid range of QM delta values to -128..127
It is not hard for the encoder to shift the quantization matrix (QM)
coefficient delta values to the range of -128..127 to shorten the svlc()
code, so we should require the smaller valid range.
Remove the symbolic constant NUM_QM_VALS. It actually obscured the code
because we need to know its value to understand the code. Also, since QM
coefficients cannot be zero, strictly speaking the number of QM values
is 255, not 256.
Check for the special value that indicates repeating coefficients after
the % 256 operation.
Issue: https://gitlab.com/AOMediaCodec/avm/-/issues/760
diff --git a/av1/common/av1_common_int.h b/av1/common/av1_common_int.h
index a53d8a4..ce1a22c 100644
--- a/av1/common/av1_common_int.h
+++ b/av1/common/av1_common_int.h
@@ -85,8 +85,6 @@
#define MAX_NUM_TEMPORAL_LAYERS 8
#define MAX_NUM_SPATIAL_LAYERS 4
-#define NUM_QM_VALS 256
-
/* clang-format off */
// clang-format seems to think this is a pointer dereference and not a
// multiplication.
diff --git a/av1/decoder/decodeframe.c b/av1/decoder/decodeframe.c
index 3c52218..2b6ac21 100644
--- a/av1/decoder/decodeframe.c
+++ b/av1/decoder/decodeframe.c
@@ -6697,7 +6697,7 @@
qm_val_t *mat = fund_mat[t][level][c];
bool coef_repeat_until_end = false;
- int16_t prev = 32;
+ qm_val_t prev = 32;
for (int i = 0; i < tx_size_2d[tsize]; i++) {
const int pos = s->scan[i];
if (tsize == TX_8X8 && qm_8x8_is_symmetric) {
@@ -6712,17 +6712,18 @@
if (!coef_repeat_until_end) {
const int32_t delta = aom_rb_read_svlc(rb);
// The valid range of quantization matrix coefficients is 1..255.
- // Therefore the valid range of delta values is -254..254.
- if (delta < -254 || delta > 254) {
+ // Therefore the valid range of delta values is -254..254. Because of
+ // the % 256 operation, the valid range of delta values can be reduced
+ // to -128..127 to shorten the svlc() code.
+ if (delta < -128 || delta > 127) {
aom_internal_error(error_info, AOM_CODEC_CORRUPT_FRAME,
"Invalid matrix_coef_delta: %d", delta);
}
- const int32_t coef = prev + delta;
- if (coef == 0 || coef == 256) {
+ const qm_val_t coef = (prev + delta + 256) % 256;
+ if (coef == 0) {
coef_repeat_until_end = true;
} else {
- prev = (coef + NUM_QM_VALS) % NUM_QM_VALS;
- assert(prev >= 1);
+ prev = coef;
}
}
mat[pos] = prev;
diff --git a/av1/encoder/bitstream.c b/av1/encoder/bitstream.c
index ba20b2c..67858aa 100644
--- a/av1/encoder/bitstream.c
+++ b/av1/encoder/bitstream.c
@@ -5730,17 +5730,16 @@
int16_t coeff = (symbol_idx == stop_symbol_idx) ? 0 : mat[pos];
int16_t delta = coeff - prev;
// The decoder reconstructs the matrix coefficient by calculating
- // (prev + delta + NUM_QM_VALS) % NUM_QM_VALS. Therefore delta,
- // delta + NUM_QM_VALS, and delta - NUM_QM_VALS are all equivalent
- // because they are equal modulo NUM_QM_VALS. If delta + NUM_QM_VALS or
- // delta - NUM_QM_VALS has a smaller absolute value than delta, it is
- // likely to have a shorter svlc() code, so we will write it instead.
- // In other words, for each delta value, we aim to find an equivalent
- // value (modulo NUM_QM_VALS) that has the shortest svlc() code.
- if (delta < -(NUM_QM_VALS / 2)) {
- delta += NUM_QM_VALS;
- } else if (delta >= NUM_QM_VALS / 2) {
- delta -= NUM_QM_VALS;
+ // (prev + delta + 256) % 256. Therefore delta, delta + 256, and
+ // delta - 256 are all equivalent because they are equal modulo 256. If
+ // delta + 256 or delta - 256 has a smaller absolute value than delta,
+ // it is likely to have a shorter svlc() code, so we will write it
+ // instead. In other words, for each delta value, we aim to find an
+ // equivalent value (modulo 256) that has the shortest svlc() code.
+ if (delta < -128) {
+ delta += 256;
+ } else if (delta > 127) {
+ delta -= 256;
}
aom_wb_write_svlc(wb, delta);
if (symbol_idx == stop_symbol_idx) {
