Commit 40a42d4b authored by David Barker's avatar David Barker Committed by Debargha Mukherjee

Fix for the use of prev frame mvs when ext-refs is on

Also fix a mismatch for resizing with ext-refs enabled.

There are various preconditions which need to be true for it to
be valid to set cm->use_prev_frame_mvs = 1, including that the
sizes of this frame and cm->prev_frame must be equal.

With ext-refs enabled, we would sometimes decide to change
cm->prev_frame to point to the LAST_FRAME reference, without
re-checking the preconditions. If the LAST_FRAME was smaller
than the current frame, this could lead to reading garbage off the
end of its mv array, and eventually to an encode/decode mismatch.

We fix this by rewriting the preconditions as checks on
cm->prev_frame directly (rather than using cm->last_width and
cm->last_height), and by testing the preconditions after any
possible adjustment.

This should not affect the bitstream unless ext-refs is enabled,
but may affect the bitstream with ext-refs even if resizing is
not used.

BUG=aomedia:521

Change-Id: I7dfd9ba82cdf77acc2e27e0c9f3aee21d6afeb54
parent ad8be034
...@@ -4884,6 +4884,9 @@ void av1_decode_frame(AV1Decoder *pbi, const uint8_t *data, ...@@ -4884,6 +4884,9 @@ void av1_decode_frame(AV1Decoder *pbi, const uint8_t *data,
uint8_t clear_data[MAX_AV1_HEADER_SIZE]; uint8_t clear_data[MAX_AV1_HEADER_SIZE];
size_t first_partition_size; size_t first_partition_size;
YV12_BUFFER_CONFIG *new_fb; YV12_BUFFER_CONFIG *new_fb;
#if CONFIG_EXT_REFS || CONFIG_TEMPMV_SIGNALING
RefBuffer *last_fb_ref_buf = &cm->frame_refs[LAST_FRAME - LAST_FRAME];
#endif // CONFIG_EXT_REFS || CONFIG_TEMPMV_SIGNALING
#if CONFIG_ADAPT_SCAN #if CONFIG_ADAPT_SCAN
av1_deliver_eob_threshold(cm, xd); av1_deliver_eob_threshold(cm, xd);
...@@ -4945,26 +4948,7 @@ void av1_decode_frame(AV1Decoder *pbi, const uint8_t *data, ...@@ -4945,26 +4948,7 @@ void av1_decode_frame(AV1Decoder *pbi, const uint8_t *data,
cm->setup_mi(cm); cm->setup_mi(cm);
#if CONFIG_TEMPMV_SIGNALING #if CONFIG_EXT_REFS || CONFIG_TEMPMV_SIGNALING
RefBuffer *last_ref_buf = &cm->frame_refs[LAST_FRAME - LAST_FRAME];
if (last_ref_buf->idx != INVALID_IDX) {
cm->prev_frame = &cm->buffer_pool->frame_bufs[last_ref_buf->idx];
if (cm->use_prev_frame_mvs) {
assert(!cm->error_resilient_mode &&
cm->width == last_ref_buf->buf->y_width &&
cm->height == last_ref_buf->buf->y_height &&
!cm->prev_frame->intra_only);
}
} else {
assert(cm->use_prev_frame_mvs == 0);
}
#else
cm->use_prev_frame_mvs =
!cm->error_resilient_mode && cm->width == cm->last_width &&
cm->height == cm->last_height && !cm->last_intra_only &&
cm->last_show_frame && (cm->last_frame_type != KEY_FRAME);
#endif
#if CONFIG_EXT_REFS
// NOTE(zoeliu): As cm->prev_frame can take neither a frame of // NOTE(zoeliu): As cm->prev_frame can take neither a frame of
// show_exisiting_frame=1, nor can it take a frame not used as // show_exisiting_frame=1, nor can it take a frame not used as
// a reference, it is probable that by the time it is being // a reference, it is probable that by the time it is being
...@@ -4975,12 +4959,28 @@ void av1_decode_frame(AV1Decoder *pbi, const uint8_t *data, ...@@ -4975,12 +4959,28 @@ void av1_decode_frame(AV1Decoder *pbi, const uint8_t *data,
// (1) Simply disable the use of previous frame mvs; or // (1) Simply disable the use of previous frame mvs; or
// (2) Have cm->prev_frame point to one reference frame buffer, // (2) Have cm->prev_frame point to one reference frame buffer,
// e.g. LAST_FRAME. // e.g. LAST_FRAME.
if (cm->use_prev_frame_mvs && !dec_is_ref_frame_buf(pbi, cm->prev_frame)) { if (!dec_is_ref_frame_buf(pbi, cm->prev_frame)) {
// Reassign the LAST_FRAME buffer to cm->prev_frame. // Reassign the LAST_FRAME buffer to cm->prev_frame.
RefBuffer *last_fb_ref_buf = &cm->frame_refs[LAST_FRAME - LAST_FRAME]; cm->prev_frame = last_fb_ref_buf->idx != INVALID_IDX
cm->prev_frame = &cm->buffer_pool->frame_bufs[last_fb_ref_buf->idx]; ? &cm->buffer_pool->frame_bufs[last_fb_ref_buf->idx]
: NULL;
} }
#endif // CONFIG_EXT_REFS #endif // CONFIG_EXT_REFS || CONFIG_TEMPMV_SIGNALING
#if CONFIG_TEMPMV_SIGNALING
if (cm->use_prev_frame_mvs) {
assert(!cm->error_resilient_mode && !cm->prev_frame &&
cm->width == last_fb_ref_buf->buf->y_width &&
cm->height == last_fb_ref_buf->buf->y_height &&
!cm->prev_frame->intra_only);
}
#else
cm->use_prev_frame_mvs = !cm->error_resilient_mode && cm->prev_frame &&
cm->width == cm->prev_frame->buf.y_crop_width &&
cm->height == cm->prev_frame->buf.y_crop_height &&
!cm->last_intra_only && cm->last_show_frame &&
(cm->last_frame_type != KEY_FRAME);
#endif // CONFIG_TEMPMV_SIGNALING
av1_setup_block_planes(xd, cm->subsampling_x, cm->subsampling_y); av1_setup_block_planes(xd, cm->subsampling_x, cm->subsampling_y);
......
...@@ -5321,23 +5321,8 @@ static void encode_frame_internal(AV1_COMP *cpi) { ...@@ -5321,23 +5321,8 @@ static void encode_frame_internal(AV1_COMP *cpi) {
av1_initialize_rd_consts(cpi); av1_initialize_rd_consts(cpi);
av1_initialize_me_consts(cpi, x, cm->base_qindex); av1_initialize_me_consts(cpi, x, cm->base_qindex);
init_encode_frame_mb_context(cpi); init_encode_frame_mb_context(cpi);
#if CONFIG_TEMPMV_SIGNALING
if (last_fb_buf_idx != INVALID_IDX) {
cm->prev_frame = &cm->buffer_pool->frame_bufs[last_fb_buf_idx];
cm->use_prev_frame_mvs &= !cm->error_resilient_mode &&
cm->width == cm->prev_frame->buf.y_width &&
cm->height == cm->prev_frame->buf.y_height &&
!cm->intra_only && !cm->prev_frame->intra_only;
} else {
cm->use_prev_frame_mvs = 0;
}
#else
cm->use_prev_frame_mvs =
!cm->error_resilient_mode && cm->width == cm->last_width &&
cm->height == cm->last_height && !cm->intra_only && cm->last_show_frame;
#endif
#if CONFIG_EXT_REFS #if CONFIG_EXT_REFS || CONFIG_TEMPMV_SIGNALING
// NOTE(zoeliu): As cm->prev_frame can take neither a frame of // NOTE(zoeliu): As cm->prev_frame can take neither a frame of
// show_exisiting_frame=1, nor can it take a frame not used as // show_exisiting_frame=1, nor can it take a frame not used as
// a reference, it is probable that by the time it is being // a reference, it is probable that by the time it is being
...@@ -5348,11 +5333,29 @@ static void encode_frame_internal(AV1_COMP *cpi) { ...@@ -5348,11 +5333,29 @@ static void encode_frame_internal(AV1_COMP *cpi) {
// (1) Simply disable the use of previous frame mvs; or // (1) Simply disable the use of previous frame mvs; or
// (2) Have cm->prev_frame point to one reference frame buffer, // (2) Have cm->prev_frame point to one reference frame buffer,
// e.g. LAST_FRAME. // e.g. LAST_FRAME.
if (cm->use_prev_frame_mvs && !enc_is_ref_frame_buf(cpi, cm->prev_frame)) { if (!enc_is_ref_frame_buf(cpi, cm->prev_frame)) {
// Reassign the LAST_FRAME buffer to cm->prev_frame. // Reassign the LAST_FRAME buffer to cm->prev_frame.
cm->prev_frame = &cm->buffer_pool->frame_bufs[last_fb_buf_idx]; cm->prev_frame = last_fb_buf_idx != INVALID_IDX
? &cm->buffer_pool->frame_bufs[last_fb_buf_idx]
: NULL;
} }
#endif // CONFIG_EXT_REFS #endif // CONFIG_EXT_REFS || CONFIG_TEMPMV_SIGNALING
#if CONFIG_TEMPMV_SIGNALING
if (cm->prev_frame) {
cm->use_prev_frame_mvs &= !cm->error_resilient_mode &&
cm->width == cm->prev_frame->buf.y_width &&
cm->height == cm->prev_frame->buf.y_height &&
!cm->intra_only && !cm->prev_frame->intra_only;
} else {
cm->use_prev_frame_mvs = 0;
}
#else
cm->use_prev_frame_mvs = !cm->error_resilient_mode && cm->prev_frame &&
cm->width == cm->prev_frame->buf.y_crop_width &&
cm->height == cm->prev_frame->buf.y_crop_height &&
!cm->intra_only && cm->last_show_frame;
#endif // CONFIG_TEMPMV_SIGNALING
// Special case: set prev_mi to NULL when the previous mode info // Special case: set prev_mi to NULL when the previous mode info
// context cannot be used. // context cannot be used.
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment