Update frame size in actual encoding Issue explanation: The unit test calls set_config function twice after encoding the first frame. The first call of set_config reduces frame height, but is still within half of the first frame. The second call reduces frame height even more, making is less than half of the first frame, which according to the encoder logic, there is no valid ref frames, and this frame should be set as a forced keyframe. This leads to null pointer access in scale_factors later. Solution: To make sure the correct detection of a forced key frame, we need to update the frame width and height only when the actual encoding is performed. Bug: b:310548198 Change-Id: Ie8702eeac9917806950b7da3fe9befc2c7e34d7d
diff --git a/av1/av1_cx_iface.c b/av1/av1_cx_iface.c index 641cf7a..1b5fda7 100644 --- a/av1/av1_cx_iface.c +++ b/av1/av1_cx_iface.c
@@ -1507,7 +1507,17 @@ if (cfg->g_w != ctx->cfg.g_w || cfg->g_h != ctx->cfg.g_h) { if (cfg->g_lag_in_frames > 1 || cfg->g_pass != AOM_RC_ONE_PASS) ERROR("Cannot change width or height after initialization"); - if (!valid_ref_frame_size(ctx->cfg.g_w, ctx->cfg.g_h, cfg->g_w, cfg->g_h) || + // Note: function encoder_set_config() is allowed to be called multiple + // times. However, when the original frame width or height is less than two + // times of the new frame width or height, a forced key frame should be + // used. To make sure the correct detection of a forced key frame, we need + // to update the frame width and height only when the actual encoding is + // performed. cpi->last_coded_width and cpi->last_coded_height are used to + // track the actual coded frame size. + if ((ctx->ppi->cpi->last_coded_width && ctx->ppi->cpi->last_coded_height && + !valid_ref_frame_size(ctx->ppi->cpi->last_coded_width, + ctx->ppi->cpi->last_coded_height, cfg->g_w, + cfg->g_h)) || (initial_dimensions->width && (int)cfg->g_w > initial_dimensions->width) || (initial_dimensions->height && @@ -2939,6 +2949,9 @@ AV1_COMP *cpi_lap = ppi->cpi_lap; if (ppi->cpi == NULL) return AOM_CODEC_INVALID_PARAM; + ppi->cpi->last_coded_width = ppi->cpi->oxcf.frm_dim_cfg.width; + ppi->cpi->last_coded_height = ppi->cpi->oxcf.frm_dim_cfg.height; + if (ppi->lap_enabled && cpi_lap == NULL && ppi->cpi->oxcf.pass == AOM_RC_ONE_PASS) return AOM_CODEC_INVALID_PARAM;
diff --git a/av1/encoder/encoder.c b/av1/encoder/encoder.c index 452a3a4..edc0ef7 100644 --- a/av1/encoder/encoder.c +++ b/av1/encoder/encoder.c
@@ -2230,6 +2230,7 @@ init_motion_estimation(cpi); + int has_valid_ref_frame = 0; for (ref_frame = LAST_FRAME; ref_frame <= ALTREF_FRAME; ++ref_frame) { RefCntBuffer *const buf = get_ref_frame_buf(cm, ref_frame); if (buf != NULL) { @@ -2237,9 +2238,15 @@ av1_setup_scale_factors_for_frame(sf, buf->buf.y_crop_width, buf->buf.y_crop_height, cm->width, cm->height); + has_valid_ref_frame |= av1_is_valid_scale(sf); if (av1_is_scaled(sf)) aom_extend_frame_borders(&buf->buf, num_planes); } } + if (!frame_is_intra_only(cm) && !has_valid_ref_frame) { + aom_internal_error( + cm->error, AOM_CODEC_CORRUPT_FRAME, + "Can't find at least one reference frame with valid size"); + } av1_setup_scale_factors_for_frame(&cm->sf_identity, cm->width, cm->height, cm->width, cm->height);
diff --git a/av1/encoder/encoder.h b/av1/encoder/encoder.h index 9d02993..02c89bc 100644 --- a/av1/encoder/encoder.h +++ b/av1/encoder/encoder.h
@@ -3183,6 +3183,18 @@ int initial_mbs; /*! + * The width of the frame that is lastly encoded. + * It is updated in the function "encoder_encode()". + */ + int last_coded_width; + + /*! + * The height of the frame that is lastly encoded. + * It is updated in the function "encoder_encode()". + */ + int last_coded_height; + + /*! * Resize related parameters. */ ResizePendingParams resize_pending_params;
diff --git a/test/encode_api_test.cc b/test/encode_api_test.cc index fe0ff19..035d955 100644 --- a/test/encode_api_test.cc +++ b/test/encode_api_test.cc
@@ -320,7 +320,6 @@ return image; } -// Run this test in the debugger and set a breakpoint in aom_internal_error. TEST(EncodeAPI, Buganizer310548198) { aom_codec_iface_t *const iface = aom_codec_av1_cx(); aom_codec_enc_cfg_t cfg; @@ -352,13 +351,10 @@ } aom_img_free(image); - // Uncomment this code to reproduce the bug. -#if 0 cfg.g_w = 1; cfg.g_h = 254; ASSERT_EQ(aom_codec_enc_config_set(&enc, &cfg), AOM_CODEC_OK) << aom_codec_error_detail(&enc); -#endif cfg.g_w = 1; cfg.g_h = 154;