Skip to content

Commit

Permalink
Fix a bug in simple motion search
Browse files Browse the repository at this point in the history
This change fixed a bug revealed by b/311294795.
In simple motion search, the reference buffer pointer needs to be
restored after the search. Otherwise, it causes problems while the
reference frame scaling happens. This CL fixes the bug.

Bug: b/311294795
Change-Id: I093722d5888de3cc6a6542de82a6ec9d601f897d
(cherry picked from commit 50ed636)
  • Loading branch information
Yunqing Wang authored and wantehchang committed Dec 8, 2023
1 parent 36b2dec commit 75d7727
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 2 deletions.
31 changes: 31 additions & 0 deletions test/encode_api_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -863,6 +863,37 @@ TEST(EncodeAPI, Buganizer312875957PredBufferStride) {
encoder.Encode(false);
}

// This is a test case from clusterfuzz: based on b/311294795
// Encode a few frames with multiple change config calls
// with different frame sizes.
TEST(EncodeAPI, Buganizer311294795) {
VP9Encoder encoder(1);

// Set initial config.
encoder.Configure(12, 1678, 620, VPX_VBR, VPX_DL_REALTIME);

// Encode first frame.
encoder.Encode(false);

// Change config.
encoder.Configure(16, 632, 620, VPX_VBR, VPX_DL_GOOD_QUALITY);

// Encode 2nd frame with new config
encoder.Encode(true);

// Change config.
encoder.Configure(16, 1678, 342, VPX_VBR, VPX_DL_GOOD_QUALITY);

// Encode 3rd frame with new config.
encoder.Encode(false);

// Change config.
encoder.Configure(0, 1574, 618, VPX_VBR, VPX_DL_REALTIME);
// Encode more frames with new config.
encoder.Encode(false);
encoder.Encode(false);
}

class EncodeApiGetTplStatsTest
: public ::libvpx_test::EncoderTest,
public ::testing::TestWithParam<const libvpx_test::CodecFactory *> {
Expand Down
15 changes: 13 additions & 2 deletions vp9/encoder/vp9_encodeframe.c
Original file line number Diff line number Diff line change
Expand Up @@ -3443,11 +3443,17 @@ static void simple_motion_search(const VP9_COMP *const cpi, MACROBLOCK *const x,
MV ref_mv_full = { ref_mv.row >> 3, ref_mv.col >> 3 };
MV best_mv = { 0, 0 };
int cost_list[5];
struct buf_2d backup_pre[MAX_MB_PLANE] = { { 0, 0 } };

if (scaled_ref_frame)
if (scaled_ref_frame) {
yv12 = scaled_ref_frame;
else
// As reported in b/311294795, the reference buffer pointer needs to be
// saved and restored after the search. Otherwise, it causes problems while
// the reference frame scaling happens.
for (int i = 0; i < MAX_MB_PLANE; i++) backup_pre[i] = xd->plane[i].pre[0];
} else {
yv12 = get_ref_frame_buffer(cpi, ref);
}

assert(yv12 != NULL);
if (!yv12) return;
Expand All @@ -3464,6 +3470,11 @@ static void simple_motion_search(const VP9_COMP *const cpi, MACROBLOCK *const x,
x->mv_limits = tmp_mv_limits;
mi->mv[0].as_mv = best_mv;

// Restore reference buffer pointer.
if (scaled_ref_frame) {
for (int i = 0; i < MAX_MB_PLANE; i++) xd->plane[i].pre[0] = backup_pre[i];
}

set_ref_ptrs(cm, xd, mi->ref_frame[0], mi->ref_frame[1]);
xd->plane[0].dst.buf = pred_buf;
xd->plane[0].dst.stride = 64;
Expand Down

0 comments on commit 75d7727

Please sign in to comment.