Issue 1 - Incorrect chroma SATD for YCBCR422
The tile->rc.cost computed by v0.3.0.0 for a YCBCR422 input differs from v0.2.1.0 (using the fastest rate-control preset). Is this change intended?
/* oapve_rc_get_tile_cost */
int tx = (tile->x + x) >> ctx->c_sft[c][0];
int ty = (tile->y + y) >> ctx->c_sft[c][1];
...
ctx->fn_blk_from_pic[c](OAPV_BLK_W, OAPV_BLK_H, pic, tx, org_s,
core->coef, (OAPV_BLK_W << 1), ctx->bit_depth, 0);
With the fix above, I was able to obtain the same result as v0.2.1.0.
Issue 2 - Potential overflow in cost accumulator
The cost accumulator (sum) is currently declared as a 32-bit signed int. For larger tiles (e.g., a whole 8K frame as one tile), higher bit depth, higher-energy content, the accumulation may exceed INT_MAX.
Note the destination tile->rc.cost is already a double (see oapve_rc_tile_t in src/oapv_def.h), so widening the accumulator has no precision downside. The related aggregate oapve_rc_get_tile_cost_thread() already uses a u64* sum across tiles — the per-tile accumulator is the one still on int.
/* oapve_rc_get_tile_cost */
double sum = 0;
...
tile->rc.cost = sum;
Issue 1 - Incorrect chroma SATD for YCBCR422
The
tile->rc.costcomputed by v0.3.0.0 for a YCBCR422 input differs from v0.2.1.0 (using thefastestrate-control preset). Is this change intended?With the fix above, I was able to obtain the same result as v0.2.1.0.
Issue 2 - Potential overflow in cost accumulator
The cost accumulator (
sum) is currently declared as a 32-bit signedint. For larger tiles (e.g., a whole 8K frame as one tile), higher bit depth, higher-energy content, the accumulation may exceedINT_MAX.Note the destination
tile->rc.costis already adouble(seeoapve_rc_tile_tinsrc/oapv_def.h), so widening the accumulator has no precision downside. The related aggregateoapve_rc_get_tile_cost_thread()already uses au64* sumacross tiles — the per-tile accumulator is the one still onint.