Skip to content

Commit

Permalink
Update frame size in actual encoding
Browse files Browse the repository at this point in the history
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 1ed56a4)
  • Loading branch information
chengchen-google authored and wantehchang committed Dec 8, 2023
1 parent 75d7727 commit 6bb806b
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 2 deletions.
19 changes: 19 additions & 0 deletions test/encode_api_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions vp9/encoder/vp9_encoder.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
Expand Down
3 changes: 3 additions & 0 deletions vp9/encoder/vp9_encoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 17 additions & 2 deletions vp9/vp9_cx_iface.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 6bb806b

Please sign in to comment.