Prevent undefined behaviour for AMVR experiment
Sequences starting with intra-only frames previously resulted in undefined
behaviour with CONFIG_AMVR == 1, as seq_force_integer_mv was only read for
keyframes.
This patch makes changes as follows:
- The syntax element force_screen_content_tools has been added to the
SequenceHeader struct, and is read and written correspondingly
- seq_force_integer_mv has been renamed to force_integer_mv and moved to the
SequenceHeader struct, and is read and written correspondingly (provided that
force_screen_content_tools != 0)
- The conditional reading/writing of allow_screen_content_tools now happens for
every frame after reading/writing error_resilient_mode (CONFIG_OBU == 1) or
the sequence header (CONFIG_OBU == 0)
- The conditional reading/writing of cur_frame_force_integer_mv now happens for
every frame after reading/writing allow_screen_content_tools
BUG=aomedia:1048
Change-Id: I689476fc2fa781dc8ec6fc8da91926cc8cfd3dc2
diff --git a/av1/common/onyxc_int.h b/av1/common/onyxc_int.h
index e7520ad..de86b19 100644
--- a/av1/common/onyxc_int.h
+++ b/av1/common/onyxc_int.h
@@ -223,6 +223,14 @@
BLOCK_SIZE sb_size; // Size of the superblock used for this frame
int mib_size; // Size of the superblock in units of MI blocks
int mib_size_log2; // Log 2 of above.
+ int force_screen_content_tools; // 0 - force off
+ // 1 - force on
+ // 2 - adaptive
+#if CONFIG_AMVR
+ int force_integer_mv; // 0 - Not to force. MV can be in 1/4 or 1/8
+ // 1 - force to integer
+ // 2 - adaptive
+#endif
#if CONFIG_MONO_VIDEO
int monochrome;
#endif // CONFIG_MONO_VIDEO
@@ -314,9 +322,6 @@
int allow_high_precision_mv;
#if CONFIG_AMVR
- int seq_force_integer_mv; // 0 - Not to force. MV can be in 1/4 or 1/8
- // 1 - force to integer
- // 2 - adaptive
int cur_frame_force_integer_mv; // 0 the default in AOM, 1 only integer
#endif
diff --git a/av1/decoder/decodeframe.c b/av1/decoder/decodeframe.c
index af881e5..73aaf12 100644
--- a/av1/decoder/decodeframe.c
+++ b/av1/decoder/decodeframe.c
@@ -2438,6 +2438,24 @@
}
setup_sb_size(seq_params, rb);
+
+ if (aom_rb_read_bit(rb)) {
+ seq_params->force_screen_content_tools = 2;
+ } else {
+ seq_params->force_screen_content_tools = aom_rb_read_bit(rb);
+ }
+
+#if CONFIG_AMVR
+ if (seq_params->force_screen_content_tools > 0) {
+ if (aom_rb_read_bit(rb)) {
+ seq_params->force_integer_mv = 2;
+ } else {
+ seq_params->force_integer_mv = aom_rb_read_bit(rb);
+ }
+ } else {
+ seq_params->force_integer_mv = 2;
+ }
+#endif
}
#endif // CONFIG_REFERENCE_BUFFER || CONFIG_OBU
@@ -2756,10 +2774,30 @@
cm->intra_only = cm->frame_type == INTRA_ONLY_FRAME;
#endif
cm->error_resilient_mode = aom_rb_read_bit(rb);
+
#if CONFIG_REFERENCE_BUFFER
#if !CONFIG_OBU
if (frame_is_intra_only(cm)) read_sequence_header(&cm->seq_params, rb);
#endif // !CONFIG_OBU
+
+ if (cm->seq_params.force_screen_content_tools == 2) {
+ cm->allow_screen_content_tools = aom_rb_read_bit(rb);
+ } else {
+ cm->allow_screen_content_tools = cm->seq_params.force_screen_content_tools;
+ }
+
+#if CONFIG_AMVR
+ if (cm->allow_screen_content_tools) {
+ if (cm->seq_params.force_integer_mv == 2) {
+ cm->cur_frame_force_integer_mv = aom_rb_read_bit(rb);
+ } else {
+ cm->cur_frame_force_integer_mv = cm->seq_params.force_integer_mv;
+ }
+ } else {
+ cm->cur_frame_force_integer_mv = 0;
+ }
+#endif // CONFIG_AMVR
+
if (cm->seq_params.frame_id_numbers_present_flag) {
int frame_id_length = cm->seq_params.frame_id_length;
int diff_len = cm->seq_params.delta_frame_id_length;
@@ -2838,21 +2876,9 @@
memset(&cm->ref_frame_map, -1, sizeof(cm->ref_frame_map));
pbi->need_resync = 0;
}
- cm->allow_screen_content_tools = aom_rb_read_bit(rb);
#if CONFIG_INTRABC
if (cm->allow_screen_content_tools) cm->allow_intrabc = aom_rb_read_bit(rb);
#endif // CONFIG_INTRABC
-#if CONFIG_AMVR
- if (cm->allow_screen_content_tools) {
- if (aom_rb_read_bit(rb)) {
- cm->seq_force_integer_mv = 2;
- } else {
- cm->seq_force_integer_mv = aom_rb_read_bit(rb);
- }
- } else {
- cm->seq_force_integer_mv = 2;
- }
-#endif
cm->use_prev_frame_mvs = 0;
} else {
if (cm->intra_only || cm->error_resilient_mode) cm->use_prev_frame_mvs = 0;
@@ -2900,7 +2926,6 @@
memset(&cm->ref_frame_map, -1, sizeof(cm->ref_frame_map));
pbi->need_resync = 0;
}
- cm->allow_screen_content_tools = aom_rb_read_bit(rb);
#if CONFIG_INTRABC
if (cm->allow_screen_content_tools)
cm->allow_intrabc = aom_rb_read_bit(rb);
@@ -2972,16 +2997,6 @@
#endif
#if CONFIG_AMVR
- if (cm->allow_screen_content_tools) {
- if (cm->seq_force_integer_mv == 2) {
- cm->cur_frame_force_integer_mv = aom_rb_read_bit(rb);
- } else {
- cm->cur_frame_force_integer_mv = cm->seq_force_integer_mv;
- }
- } else {
- cm->cur_frame_force_integer_mv = 0;
- }
-
if (cm->cur_frame_force_integer_mv) {
cm->allow_high_precision_mv = 0;
} else {
diff --git a/av1/encoder/bitstream.c b/av1/encoder/bitstream.c
index ec01e8f..cf7ff27 100644
--- a/av1/encoder/bitstream.c
+++ b/av1/encoder/bitstream.c
@@ -3485,6 +3485,26 @@
}
write_sb_size(seq_params, wb);
+
+ if (seq_params->force_screen_content_tools == 2) {
+ aom_wb_write_bit(wb, 1);
+ } else {
+ aom_wb_write_bit(wb, 0);
+ aom_wb_write_bit(wb, seq_params->force_screen_content_tools);
+ }
+
+#if CONFIG_AMVR
+ if (seq_params->force_screen_content_tools > 0) {
+ if (seq_params->force_integer_mv == 2) {
+ aom_wb_write_bit(wb, 1);
+ } else {
+ aom_wb_write_bit(wb, 0);
+ aom_wb_write_bit(wb, seq_params->force_integer_mv);
+ }
+ } else {
+ assert(seq_params->force_integer_mv == 2);
+ }
+#endif
}
#endif // CONFIG_REFERENCE_BUFFER || CONFIG_OBU
@@ -3663,6 +3683,26 @@
write_sequence_header(cpi, wb);
#endif // CONFIG_REFERENCE_BUFFER
}
+
+ if (cm->seq_params.force_screen_content_tools == 2) {
+ aom_wb_write_bit(wb, cm->allow_screen_content_tools);
+ } else {
+ assert(cm->allow_screen_content_tools ==
+ cm->seq_params.force_screen_content_tools);
+ }
+
+#if CONFIG_AMVR
+ if (cm->allow_screen_content_tools) {
+ if (cm->seq_params.force_integer_mv == 2) {
+ aom_wb_write_bit(wb, cm->cur_frame_force_integer_mv);
+ } else {
+ assert(cm->cur_frame_force_integer_mv == cm->seq_params.force_integer_mv);
+ }
+ } else {
+ assert(cm->cur_frame_force_integer_mv == 0);
+ }
+#endif // CONFIG_AMVR
+
#if CONFIG_REFERENCE_BUFFER
cm->invalid_delta_frame_id_minus1 = 0;
if (cm->seq_params.frame_id_numbers_present_flag) {
@@ -3704,20 +3744,9 @@
#else
write_frame_size(cm, wb);
#endif
- aom_wb_write_bit(wb, cm->allow_screen_content_tools);
#if CONFIG_INTRABC
if (cm->allow_screen_content_tools) aom_wb_write_bit(wb, cm->allow_intrabc);
#endif // CONFIG_INTRABC
-#if CONFIG_AMVR
- if (cm->allow_screen_content_tools) {
- if (cm->seq_force_integer_mv == 2) {
- aom_wb_write_bit(wb, 1);
- } else {
- aom_wb_write_bit(wb, 0);
- aom_wb_write_bit(wb, cm->seq_force_integer_mv);
- }
- }
-#endif
} else {
#if !CONFIG_NO_FRAME_CONTEXT_SIGNALING
if (!cm->error_resilient_mode) {
@@ -3750,7 +3779,6 @@
#else
write_frame_size(cm, wb);
#endif
- aom_wb_write_bit(wb, cm->allow_screen_content_tools);
#if CONFIG_INTRABC
if (cm->allow_screen_content_tools)
aom_wb_write_bit(wb, cm->allow_intrabc);
@@ -3798,16 +3826,6 @@
#endif
#if CONFIG_AMVR
- if (cm->allow_screen_content_tools) {
- if (cm->seq_force_integer_mv == 2) {
- aom_wb_write_bit(wb, cm->cur_frame_force_integer_mv);
- } else {
- assert(cm->cur_frame_force_integer_mv == cm->seq_force_integer_mv);
- }
- } else {
- assert(cm->cur_frame_force_integer_mv == 0);
- }
-
if (cm->cur_frame_force_integer_mv) {
cm->allow_high_precision_mv = 0;
} else {
@@ -4017,6 +4035,25 @@
aom_wb_write_bit(wb, cm->show_frame);
aom_wb_write_bit(wb, cm->error_resilient_mode);
+ if (cm->seq_params.force_screen_content_tools == 2) {
+ aom_wb_write_bit(wb, cm->allow_screen_content_tools);
+ } else {
+ assert(cm->allow_screen_content_tools ==
+ cm->seq_params.force_screen_content_tools);
+ }
+
+#if CONFIG_AMVR
+ if (cm->allow_screen_content_tools) {
+ if (cm->seq_params.force_integer_mv == 2) {
+ aom_wb_write_bit(wb, cm->cur_frame_force_integer_mv);
+ } else {
+ assert(cm->cur_frame_force_integer_mv == cm->seq_params.force_integer_mv);
+ }
+ } else {
+ assert(cm->cur_frame_force_integer_mv == 0);
+ }
+#endif // CONFIG_AMVR
+
#if CONFIG_REFERENCE_BUFFER
cm->invalid_delta_frame_id_minus1 = 0;
if (cm->seq_params.frame_id_numbers_present_flag) {
@@ -4043,20 +4080,9 @@
#else
write_frame_size(cm, wb);
#endif
- aom_wb_write_bit(wb, cm->allow_screen_content_tools);
#if CONFIG_INTRABC
if (cm->allow_screen_content_tools) aom_wb_write_bit(wb, cm->allow_intrabc);
#endif // CONFIG_INTRABC
-#if CONFIG_AMVR
- if (cm->allow_screen_content_tools) {
- if (cm->seq_force_integer_mv == 2) {
- aom_wb_write_bit(wb, 1);
- } else {
- aom_wb_write_bit(wb, 0);
- aom_wb_write_bit(wb, cm->seq_force_integer_mv == 0);
- }
- }
-#endif
} else if (cm->frame_type == INTRA_ONLY_FRAME) {
#if !CONFIG_NO_FRAME_CONTEXT_SIGNALING
if (!cm->error_resilient_mode) {
@@ -4075,7 +4101,6 @@
#else
write_frame_size(cm, wb);
#endif
- aom_wb_write_bit(wb, cm->allow_screen_content_tools);
#if CONFIG_INTRABC
if (cm->allow_screen_content_tools)
aom_wb_write_bit(wb, cm->allow_intrabc);
@@ -4134,16 +4159,6 @@
#endif
#if CONFIG_AMVR
- if (cm->allow_screen_content_tools) {
- if (cm->seq_force_integer_mv == 2) {
- aom_wb_write_bit(wb, cm->cur_frame_force_integer_mv);
- } else {
- assert(cm->cur_frame_force_integer_mv == cm->seq_force_integer_mv);
- }
- } else {
- assert(cm->cur_frame_force_integer_mv == 0);
- }
-
if (cm->cur_frame_force_integer_mv) {
cm->allow_high_precision_mv = 0;
} else {
diff --git a/av1/encoder/encodeframe.c b/av1/encoder/encodeframe.c
index c635234..85464ab 100644
--- a/av1/encoder/encodeframe.c
+++ b/av1/encoder/encodeframe.c
@@ -4244,12 +4244,17 @@
av1_zero(rdc->comp_pred_diff);
if (frame_is_intra_only(cm)) {
- cm->allow_screen_content_tools =
- cpi->oxcf.content == AOM_CONTENT_SCREEN ||
- is_screen_content(cpi->source->y_buffer,
- cpi->source->flags & YV12_FLAG_HIGHBITDEPTH, xd->bd,
- cpi->source->y_stride, cpi->source->y_width,
- cpi->source->y_height);
+ if (cm->seq_params.force_screen_content_tools == 2) {
+ cm->allow_screen_content_tools =
+ cpi->oxcf.content == AOM_CONTENT_SCREEN ||
+ is_screen_content(cpi->source->y_buffer,
+ cpi->source->flags & YV12_FLAG_HIGHBITDEPTH, xd->bd,
+ cpi->source->y_stride, cpi->source->y_width,
+ cpi->source->y_height);
+ } else {
+ cm->allow_screen_content_tools =
+ cm->seq_params.force_screen_content_tools;
+ }
}
#if CONFIG_INTRABC
diff --git a/av1/encoder/encoder.c b/av1/encoder/encoder.c
index 92ee64c..06703c7 100644
--- a/av1/encoder/encoder.c
+++ b/av1/encoder/encoder.c
@@ -3261,8 +3261,10 @@
cpi->ext_refresh_frame_context_pending = 0;
highbd_set_var_fns(cpi);
+
+ cm->seq_params.force_screen_content_tools = 2;
#if CONFIG_AMVR
- cm->seq_force_integer_mv = 2;
+ cm->seq_params.force_integer_mv = 2;
#endif
}
@@ -7011,13 +7013,14 @@
#if CONFIG_AMVR
cpi->cur_poc++;
if (oxcf->pass != 1 && cpi->common.allow_screen_content_tools) {
- if (cpi->common.seq_force_integer_mv == 2) {
+ if (cpi->common.seq_params.force_integer_mv == 2) {
struct lookahead_entry *previous_entry =
cpi->lookahead->buf + cpi->previsous_index;
cpi->common.cur_frame_force_integer_mv = is_integer_mv(
cpi, cpi->source, &previous_entry->img, cpi->previsou_hash_table);
} else {
- cpi->common.cur_frame_force_integer_mv = cpi->common.seq_force_integer_mv;
+ cpi->common.cur_frame_force_integer_mv =
+ cpi->common.seq_params.force_integer_mv;
}
} else {
cpi->common.cur_frame_force_integer_mv = 0;
