aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCheng Chen <chengchen@google.com>2023-12-04 15:34:50 -0800
committerWan-Teh Chang <wtc@google.com>2023-12-08 21:34:57 +0000
commit6bb806b177b133edde6be45fadfa3e0d453376f3 (patch)
treea0fa3535e0b4421cd63a544f295dff3daf8cbd55
parent75d7727f58ddf751b4333a4df94cab1fd920a35c (diff)
downloadlibvpx-6bb806b177b133edde6be45fadfa3e0d453376f3.tar.gz
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 width, but is still within half of the first frame. The second call reduces frame width 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/311985118 Change-Id: Ie2cd3b760d4a4b399845693d7421c4eb11a12775 (cherry picked from commit 1ed56a46b3f6b18e1fb89a091e60d80ae20eec01)
-rw-r--r--test/encode_api_test.cc19
-rw-r--r--vp9/encoder/vp9_encoder.c7
-rw-r--r--vp9/encoder/vp9_encoder.h3
-rw-r--r--vp9/vp9_cx_iface.c19
4 files changed, 46 insertions, 2 deletions
diff --git a/test/encode_api_test.cc b/test/encode_api_test.cc
index e2374abb4..0aa8739d6 100644
--- a/test/encode_api_test.cc
+++ b/test/encode_api_test.cc
@@ -801,6 +801,25 @@ TEST(EncodeAPI, Buganizer311394513) {
encoder.Encode(true);
}
+TEST(EncodeAPI, Buganizer311985118) {
+ VP9Encoder encoder(0);
+
+ // Set initial config, in particular set deadline to GOOD mode.
+ encoder.Configure(12, 1678, 620, VPX_VBR, VPX_DL_GOOD_QUALITY);
+
+ // Encode 1st frame.
+ encoder.Encode(false);
+
+ // Change config: change threads and width.
+ encoder.Configure(0, 1574, 620, VPX_VBR, VPX_DL_GOOD_QUALITY);
+
+ // Change config: change threads, width and height.
+ encoder.Configure(16, 837, 432, VPX_VBR, VPX_DL_GOOD_QUALITY);
+
+ // Encode 2nd frame.
+ encoder.Encode(false);
+}
+
// This is a test case from clusterfuzz: based on b/314857577.
// Encode a few frames with multiple change config calls
// with different frame sizes.
diff --git a/vp9/encoder/vp9_encoder.c b/vp9/encoder/vp9_encoder.c
index 20e35077c..152d42bc9 100644
--- a/vp9/encoder/vp9_encoder.c
+++ b/vp9/encoder/vp9_encoder.c
@@ -3888,6 +3888,7 @@ static void set_frame_size(VP9_COMP *cpi) {
alloc_util_frame_buffers(cpi);
init_motion_estimation(cpi);
+ int has_valid_ref_frame = 0;
for (ref_frame = LAST_FRAME; ref_frame <= ALTREF_FRAME; ++ref_frame) {
RefBuffer *const ref_buf = &cm->frame_refs[ref_frame - 1];
const int buf_idx = get_ref_frame_buf_idx(cpi, ref_frame);
@@ -3906,11 +3907,17 @@ static void set_frame_size(VP9_COMP *cpi) {
buf->y_crop_height, cm->width,
cm->height);
#endif // CONFIG_VP9_HIGHBITDEPTH
+ has_valid_ref_frame |= vp9_is_valid_scale(&ref_buf->sf);
if (vp9_is_scaled(&ref_buf->sf)) vpx_extend_frame_borders(buf);
} else {
ref_buf->buf = NULL;
}
}
+ if (!frame_is_intra_only(cm) && !has_valid_ref_frame) {
+ vpx_internal_error(
+ &cm->error, VPX_CODEC_CORRUPT_FRAME,
+ "Can't find at least one reference frame with valid size");
+ }
set_ref_ptrs(cm, xd, LAST_FRAME, LAST_FRAME);
}
diff --git a/vp9/encoder/vp9_encoder.h b/vp9/encoder/vp9_encoder.h
index 83b7081e7..7136f7faa 100644
--- a/vp9/encoder/vp9_encoder.h
+++ b/vp9/encoder/vp9_encoder.h
@@ -921,6 +921,9 @@ typedef struct VP9_COMP {
// number of MBs in the current frame when the frame is
// scaled.
+ int last_coded_width;
+ int last_coded_height;
+
int use_svc;
SVC svc;
diff --git a/vp9/vp9_cx_iface.c b/vp9/vp9_cx_iface.c
index b75fc18fd..d4c41d16f 100644
--- a/vp9/vp9_cx_iface.c
+++ b/vp9/vp9_cx_iface.c
@@ -797,10 +797,22 @@ static vpx_codec_err_t encoder_set_config(vpx_codec_alg_priv_t *ctx,
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 != VPX_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->cpi->last_coded_width && ctx->cpi->last_coded_height &&
+ !valid_ref_frame_size(ctx->cpi->last_coded_width,
+ ctx->cpi->last_coded_height, cfg->g_w,
+ cfg->g_h)) ||
(ctx->cpi->initial_width && (int)cfg->g_w > ctx->cpi->initial_width) ||
- (ctx->cpi->initial_height && (int)cfg->g_h > ctx->cpi->initial_height))
+ (ctx->cpi->initial_height &&
+ (int)cfg->g_h > ctx->cpi->initial_height)) {
force_key = 1;
+ }
}
// Prevent increasing lag_in_frames. This check is stricter than it needs
@@ -1310,6 +1322,9 @@ static vpx_codec_err_t encoder_encode(vpx_codec_alg_priv_t *ctx,
if (cpi == NULL) return VPX_CODEC_INVALID_PARAM;
+ cpi->last_coded_width = ctx->oxcf.width;
+ cpi->last_coded_height = ctx->oxcf.height;
+
if (img != NULL) {
res = validate_img(ctx, img);
if (res == VPX_CODEC_OK) {