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)
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.
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);
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);
}
// 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;
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
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) {