Fix `upsample_pred` buffer allocation size The size allocated to the buffer `upsample_pred` which is used to store the intermediate data after horizontal filtering is corrected. Additionally, this patch corrects the related assertion checks and adds comments to clarify the buffer usage. Change-Id: I8089e130adfb62db23e62f1053f485daefaf0b63
diff --git a/av1/encoder/arm/reconinter_enc_neon.c b/av1/encoder/arm/reconinter_enc_neon.c index ac6501f..5c12301 100644 --- a/av1/encoder/arm/reconinter_enc_neon.c +++ b/av1/encoder/arm/reconinter_enc_neon.c
@@ -88,7 +88,7 @@ const int ref_vert_offset = ref_stride * ((SUBPEL_TAPS >> 1) - 1); const int im_vert_offset = im_stride * ((filter_params->taps >> 1) - 1); - assert(im_height <= (MAX_SB_SIZE * 2 + 16) + 16); + assert(im_height < (MAX_SB_SIZE + SUBPEL_TAPS)); aom_convolve8_horiz(ref - ref_vert_offset, ref_stride, im_block, MAX_SB_SIZE, filter_x, 16, NULL, -1, width, im_height); aom_convolve8_vert(im_block + im_vert_offset, MAX_SB_SIZE, comp_pred, width, @@ -178,7 +178,7 @@ av1_get_interp_filter_subpel_kernel(filter, subpel_y_q3 << 1); const int intermediate_height = (((height - 1) * 8 + subpel_y_q3) >> 3) + filter->taps; - assert(intermediate_height <= (MAX_SB_SIZE * 2 + 16) + 16); + assert(intermediate_height < (MAX_SB_SIZE + SUBPEL_TAPS)); aom_highbd_convolve8_horiz_neon( CONVERT_TO_BYTEPTR(ref - ref_stride * ((filter->taps >> 1) - 1)), ref_stride, CONVERT_TO_BYTEPTR(temp), MAX_SB_SIZE, kernel_x, 16, NULL,
diff --git a/av1/encoder/encoder.c b/av1/encoder/encoder.c index 3842190..ccde441 100644 --- a/av1/encoder/encoder.c +++ b/av1/encoder/encoder.c
@@ -979,9 +979,13 @@ } if (x->upsample_pred == NULL) { + // The buffer 'upsampled_pred' is used to store the output of horizontal + // filtering in aom_(highbd_)upsampled_pred() function. As the length of the + // interpolation filter used is SUBPEL_TAPS a buffer size of (MAX_SB_SIZE + + // SUBPEL_TAPS) * MAX_SB_SIZE is allocated. CHECK_MEM_ERROR( cm, x->upsample_pred, - aom_memalign(16, (1 + is_highbitdepth) * ((MAX_SB_SIZE + 16) + 16) * + aom_memalign(16, (1 + is_highbitdepth) * (MAX_SB_SIZE + SUBPEL_TAPS) * MAX_SB_SIZE * sizeof(*x->upsample_pred))); x->e_mbd.tmp_upsample_pred = x->upsample_pred; }
diff --git a/av1/encoder/ethread.c b/av1/encoder/ethread.c index d46b3ee..22d7de7 100644 --- a/av1/encoder/ethread.c +++ b/av1/encoder/ethread.c
@@ -996,7 +996,7 @@ AOM_CHECK_MEM_ERROR( &ppi->error, td->upsample_pred, - aom_memalign(16, (1 + is_highbitdepth) * ((MAX_SB_SIZE + 16) + 16) * + aom_memalign(16, (1 + is_highbitdepth) * (MAX_SB_SIZE + SUBPEL_TAPS) * MAX_SB_SIZE * sizeof(*td->upsample_pred))); if (!is_first_pass && i < num_enc_workers) {
diff --git a/av1/encoder/reconinter_enc.c b/av1/encoder/reconinter_enc.c index f169721..825e17c 100644 --- a/av1/encoder/reconinter_enc.c +++ b/av1/encoder/reconinter_enc.c
@@ -491,7 +491,7 @@ av1_get_interp_filter_subpel_kernel(filter, subpel_y_q3 << 1); const int intermediate_height = (((height - 1) * 8 + subpel_y_q3) >> 3) + filter->taps; - assert(intermediate_height <= (MAX_SB_SIZE * 2 + 16) + 16); + assert(intermediate_height < (MAX_SB_SIZE + SUBPEL_TAPS)); aom_convolve8_horiz_c(ref - ref_stride * ((filter->taps >> 1) - 1), ref_stride, temp, MAX_SB_SIZE, kernel_x, 16, NULL, -1, width, intermediate_height); @@ -574,7 +574,7 @@ av1_get_interp_filter_subpel_kernel(filter, subpel_y_q3 << 1); const int intermediate_height = (((height - 1) * 8 + subpel_y_q3) >> 3) + filter->taps; - assert(intermediate_height <= (MAX_SB_SIZE * 2 + 16) + 16); + assert(intermediate_height < (MAX_SB_SIZE + SUBPEL_TAPS)); aom_highbd_convolve8_horiz_c( CONVERT_TO_BYTEPTR(ref - ref_stride * ((filter->taps >> 1) - 1)), ref_stride, CONVERT_TO_BYTEPTR(temp), MAX_SB_SIZE, kernel_x, 16, NULL,
diff --git a/av1/encoder/x86/reconinter_enc_sse2.c b/av1/encoder/x86/reconinter_enc_sse2.c index e67ac1b..55a3ee9 100644 --- a/av1/encoder/x86/reconinter_enc_sse2.c +++ b/av1/encoder/x86/reconinter_enc_sse2.c
@@ -111,7 +111,7 @@ uint8_t *temp_start_vert = temp + MAX_SB_SIZE * ((filter->taps >> 1) - 1); int intermediate_height = (((height - 1) * 8 + subpel_y_q3) >> 3) + filter_taps; - assert(intermediate_height <= (MAX_SB_SIZE * 2 + 16) + 16); + assert(intermediate_height < (MAX_SB_SIZE + SUBPEL_TAPS)); aom_convolve8_horiz(ref_start, ref_stride, temp_start_horiz, MAX_SB_SIZE, kernel_x, 16, NULL, -1, width, intermediate_height); aom_convolve8_vert(temp_start_vert, MAX_SB_SIZE, comp_pred, width, NULL, -1, @@ -175,6 +175,17 @@ aom_highbd_convolve8_vert(ref8, ref_stride, comp_pred8, width, NULL, -1, kernel, 16, width, height, bd); } else { + // The src and dst parameters of 'aom_highbd_convolve8_vert()' call are the + // same buffer ('comp_pred8') although they start from different offsets in + // the buffer. For any given row 'y', the 8-tap vertical filter reads a + // window of input rows from 'y-3' to 'y+4' (stride = MAX_SB_SIZE) and the + // result is written to row 'y-3' (stride = width) of the `comp_pred8` + // buffer. Since 'width <= MAX_SB_SIZE', output is written only after its + // corresponding input has already been read, eliminating any risk of + // overwriting data required for future iterations. The function + // 'aom_highbd_convolve8_vert()' must process the rows from top to bottom in + // increading order of row index, otherwise it will break the in-place + // processing of 'comp_pred8'. uint16_t *temp = CONVERT_TO_SHORTPTR(comp_pred8); const uint16_t *ref = CONVERT_TO_SHORTPTR(ref8); const int16_t *const kernel_x = @@ -188,7 +199,7 @@ uint16_t *temp_start_vert = temp + MAX_SB_SIZE * ((filter->taps >> 1) - 1); const int intermediate_height = (((height - 1) * 8 + subpel_y_q3) >> 3) + filter_taps; - assert(intermediate_height <= (MAX_SB_SIZE * 2 + 16) + 16); + assert(intermediate_height < (MAX_SB_SIZE + SUBPEL_TAPS)); aom_highbd_convolve8_horiz(CONVERT_TO_BYTEPTR(ref_start), ref_stride, CONVERT_TO_BYTEPTR(temp_start_horiz), MAX_SB_SIZE, kernel_x, 16, NULL, -1, width,
diff --git a/test/comp_mask_pred_test.cc b/test/comp_mask_pred_test.cc index f57160c..be6a1fb 100644 --- a/test/comp_mask_pred_test.cc +++ b/test/comp_mask_pred_test.cc
@@ -82,10 +82,10 @@ rnd_.Reset(libaom_test::ACMRandom::DeterministicSeed()); av1_init_wedge_masks(); comp_pred1_ = - (uint8_t *)aom_memalign(16, ((MAX_SB_SIZE + 16) + 16) * MAX_SB_SIZE); + (uint8_t *)aom_memalign(16, (MAX_SB_SIZE + SUBPEL_TAPS) * MAX_SB_SIZE); ASSERT_NE(comp_pred1_, nullptr); comp_pred2_ = - (uint8_t *)aom_memalign(16, ((MAX_SB_SIZE + 16) + 16) * MAX_SB_SIZE); + (uint8_t *)aom_memalign(16, (MAX_SB_SIZE + SUBPEL_TAPS) * MAX_SB_SIZE); ASSERT_NE(comp_pred2_, nullptr); pred_ = (uint8_t *)aom_memalign(16, MAX_SB_SQUARE); ASSERT_NE(pred_, nullptr); @@ -458,10 +458,10 @@ av1_init_wedge_masks(); comp_pred1_ = (uint16_t *)aom_memalign( - 16, ((MAX_SB_SIZE + 16) + 16) * MAX_SB_SIZE * sizeof(*comp_pred1_)); + 16, (MAX_SB_SIZE + SUBPEL_TAPS) * MAX_SB_SIZE * sizeof(*comp_pred1_)); ASSERT_NE(comp_pred1_, nullptr); comp_pred2_ = (uint16_t *)aom_memalign( - 16, ((MAX_SB_SIZE + 16) + 16) * MAX_SB_SIZE * sizeof(*comp_pred2_)); + 16, (MAX_SB_SIZE + SUBPEL_TAPS) * MAX_SB_SIZE * sizeof(*comp_pred2_)); ASSERT_NE(comp_pred2_, nullptr); pred_ = (uint16_t *)aom_memalign(16, MAX_SB_SQUARE * sizeof(*pred_)); ASSERT_NE(pred_, nullptr);