From: Urvang Joshi Date: Thu, 3 Jan 2019 22:49:18 +0000 (-0800) Subject: VP9 firstpass: Bugfix when mi_col_start/end is odd X-Git-Tag: v1.8.0~16^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ad57c72b9f2e760c0e594d4711edbab39feb3b31;p=libvpx VP9 firstpass: Bugfix when mi_col_start/end is odd Before this patch, if mi_col_end was odd, then the for loop for 'mb_col' was looping once LESS than it should have been. For example, if mi_col_end = 47, then the loop was terminating when mb_col == 23. However, the correct behavior would be to terminate when mb_col == 24. The issue was introduced in: https://chromium-review.googlesource.com/c/webm/libvpx/+/423279 This can lead to many of the stats being inaccurate, for such videos (with mi_col_start/end having an odd value). As an example: Even for very static content, fp_acc_data->intercount can never reach the same value as num_mbs. And in turn, pcnt_inter can never reach the value 1 (that is, 100%). This would lead to very static videos NOT being marked static, and encoded like regular videos. Note: this is just one possible effect based on observation. Other issues are also possible based on other stats. Improvement on some test clips: ------------------------------- - One test clip saw a gain of -2.580% in VBR mode (and -3.153% in Q mode). The reason for improvement: a wrongly detected scene cut was avoided due to corrected value in 'this_frame->pcnt_inter'. - Some very static clips correctly marked as having 100% zero motion. This avoided addition of unncecessary alt-refs, thereby reducing the bitrate. BDRate (PSNR) on regular sets (VBR mode): ----------------------------------------- lowres: 0.0 midres: -0.027 (some clips were better/worse, but I double checked that changes were as expected, given correction in stats calculation). hdres: 0.0 STATS_CHANGED for the types of videos described above. Change-Id: Ifbc2c0c0815d23ec4015475680bdf8886f158dcc --- diff --git a/vp9/encoder/vp9_firstpass.c b/vp9/encoder/vp9_firstpass.c index 03ac93463..8f0da48a2 100644 --- a/vp9/encoder/vp9_firstpass.c +++ b/vp9/encoder/vp9_firstpass.c @@ -820,6 +820,8 @@ void vp9_first_pass_encode_tile_mb_row(VP9_COMP *cpi, ThreadData *td, VP9_COMMON *const cm = &cpi->common; MACROBLOCKD *const xd = &x->e_mbd; TileInfo tile = tile_data->tile_info; + const int mb_col_start = ROUND_POWER_OF_TWO(tile.mi_col_start, 1); + const int mb_col_end = ROUND_POWER_OF_TWO(tile.mi_col_end, 1); struct macroblock_plane *const p = x->plane; struct macroblockd_plane *const pd = xd->plane; const PICK_MODE_CONTEXT *ctx = &td->pc_root->none; @@ -846,9 +848,8 @@ void vp9_first_pass_encode_tile_mb_row(VP9_COMP *cpi, ThreadData *td, assert(new_yv12 != NULL); assert(frame_is_intra_only(cm) || (lst_yv12 != NULL)); - xd->mi = cm->mi_grid_visible + xd->mi_stride * (mb_row << 1) + - (tile.mi_col_start >> 1); - xd->mi[0] = cm->mi + xd->mi_stride * (mb_row << 1) + (tile.mi_col_start >> 1); + xd->mi = cm->mi_grid_visible + xd->mi_stride * (mb_row << 1) + mb_col_start; + xd->mi[0] = cm->mi + xd->mi_stride * (mb_row << 1) + mb_col_start; for (i = 0; i < MAX_MB_PLANE; ++i) { p[i].coeff = ctx->coeff_pbuf[i][1]; @@ -862,10 +863,9 @@ void vp9_first_pass_encode_tile_mb_row(VP9_COMP *cpi, ThreadData *td, uv_mb_height = 16 >> (new_yv12->y_height > new_yv12->uv_height); // Reset above block coeffs. - recon_yoffset = - (mb_row * recon_y_stride * 16) + (tile.mi_col_start >> 1) * 16; - recon_uvoffset = (mb_row * recon_uv_stride * uv_mb_height) + - (tile.mi_col_start >> 1) * uv_mb_height; + recon_yoffset = (mb_row * recon_y_stride * 16) + mb_col_start * 16; + recon_uvoffset = + (mb_row * recon_uv_stride * uv_mb_height) + mb_col_start * uv_mb_height; // Set up limit values for motion vectors to prevent them extending // outside the UMV borders. @@ -873,8 +873,7 @@ void vp9_first_pass_encode_tile_mb_row(VP9_COMP *cpi, ThreadData *td, x->mv_limits.row_max = ((cm->mb_rows - 1 - mb_row) * 16) + BORDER_MV_PIXELS_B16; - for (mb_col = tile.mi_col_start >> 1, c = 0; mb_col < (tile.mi_col_end >> 1); - ++mb_col, c++) { + for (mb_col = mb_col_start, c = 0; mb_col < mb_col_end; ++mb_col, c++) { int this_error; int this_intra_error; const int use_dc_pred = (mb_col || mb_row) && (!mb_col || !mb_row); @@ -920,7 +919,7 @@ void vp9_first_pass_encode_tile_mb_row(VP9_COMP *cpi, ThreadData *td, x->skip_encode = 0; x->fp_src_pred = 0; // Do intra prediction based on source pixels for tile boundaries - if ((mb_col == (tile.mi_col_start >> 1)) && mb_col != 0) { + if (mb_col == mb_col_start && mb_col != 0) { xd->left_mi = &mi_left; x->fp_src_pred = 1; } @@ -1310,7 +1309,7 @@ void vp9_first_pass_encode_tile_mb_row(VP9_COMP *cpi, ThreadData *td, recon_uvoffset += uv_mb_height; // Accumulate row level stats to the corresponding tile stats - if (cpi->row_mt && mb_col == (tile.mi_col_end >> 1) - 1) + if (cpi->row_mt && mb_col == mb_col_end - 1) accumulate_fp_mb_row_stat(tile_data, fp_acc_data); (*(cpi->row_mt_sync_write_ptr))(&tile_data->row_mt_sync, mb_row, c,