Commit bf2cc016 authored by Imdad Sardharwalla's avatar Imdad Sardharwalla

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
parent 8da5addb
...@@ -223,6 +223,14 @@ typedef struct SequenceHeader { ...@@ -223,6 +223,14 @@ typedef struct SequenceHeader {
BLOCK_SIZE sb_size; // Size of the superblock used for this frame 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; // Size of the superblock in units of MI blocks
int mib_size_log2; // Log 2 of above. 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 #if CONFIG_MONO_VIDEO
int monochrome; int monochrome;
#endif // CONFIG_MONO_VIDEO #endif // CONFIG_MONO_VIDEO
...@@ -314,9 +322,6 @@ typedef struct AV1Common { ...@@ -314,9 +322,6 @@ typedef struct AV1Common {
int allow_high_precision_mv; int allow_high_precision_mv;
#if CONFIG_AMVR #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 int cur_frame_force_integer_mv; // 0 the default in AOM, 1 only integer
#endif #endif
......
...@@ -2438,6 +2438,24 @@ void read_sequence_header(SequenceHeader *seq_params, ...@@ -2438,6 +2438,24 @@ void read_sequence_header(SequenceHeader *seq_params,
} }
setup_sb_size(seq_params, rb); 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 #endif // CONFIG_REFERENCE_BUFFER || CONFIG_OBU
...@@ -2756,10 +2774,30 @@ static int read_uncompressed_header(AV1Decoder *pbi, ...@@ -2756,10 +2774,30 @@ static int read_uncompressed_header(AV1Decoder *pbi,
cm->intra_only = cm->frame_type == INTRA_ONLY_FRAME; cm->intra_only = cm->frame_type == INTRA_ONLY_FRAME;
#endif #endif
cm->error_resilient_mode = aom_rb_read_bit(rb); cm->error_resilient_mode = aom_rb_read_bit(rb);
#if CONFIG_REFERENCE_BUFFER #if CONFIG_REFERENCE_BUFFER
#if !CONFIG_OBU #if !CONFIG_OBU
if (frame_is_intra_only(cm)) read_sequence_header(&cm->seq_params, rb); if (frame_is_intra_only(cm)) read_sequence_header(&cm->seq_params, rb);
#endif // !CONFIG_OBU #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) { if (cm->seq_params.frame_id_numbers_present_flag) {
int frame_id_length = cm->seq_params.frame_id_length; int frame_id_length = cm->seq_params.frame_id_length;
int diff_len = cm->seq_params.delta_frame_id_length; int diff_len = cm->seq_params.delta_frame_id_length;
...@@ -2838,21 +2876,9 @@ static int read_uncompressed_header(AV1Decoder *pbi, ...@@ -2838,21 +2876,9 @@ static int read_uncompressed_header(AV1Decoder *pbi,
memset(&cm->ref_frame_map, -1, sizeof(cm->ref_frame_map)); memset(&cm->ref_frame_map, -1, sizeof(cm->ref_frame_map));
pbi->need_resync = 0; pbi->need_resync = 0;
} }
cm->allow_screen_content_tools = aom_rb_read_bit(rb);
#if CONFIG_INTRABC #if CONFIG_INTRABC
if (cm->allow_screen_content_tools) cm->allow_intrabc = aom_rb_read_bit(rb); if (cm->allow_screen_content_tools) cm->allow_intrabc = aom_rb_read_bit(rb);
#endif // CONFIG_INTRABC #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; cm->use_prev_frame_mvs = 0;
} else { } else {
if (cm->intra_only || cm->error_resilient_mode) cm->use_prev_frame_mvs = 0; if (cm->intra_only || cm->error_resilient_mode) cm->use_prev_frame_mvs = 0;
...@@ -2900,7 +2926,6 @@ static int read_uncompressed_header(AV1Decoder *pbi, ...@@ -2900,7 +2926,6 @@ static int read_uncompressed_header(AV1Decoder *pbi,
memset(&cm->ref_frame_map, -1, sizeof(cm->ref_frame_map)); memset(&cm->ref_frame_map, -1, sizeof(cm->ref_frame_map));
pbi->need_resync = 0; pbi->need_resync = 0;
} }
cm->allow_screen_content_tools = aom_rb_read_bit(rb);
#if CONFIG_INTRABC #if CONFIG_INTRABC
if (cm->allow_screen_content_tools) if (cm->allow_screen_content_tools)
cm->allow_intrabc = aom_rb_read_bit(rb); cm->allow_intrabc = aom_rb_read_bit(rb);
...@@ -2972,16 +2997,6 @@ static int read_uncompressed_header(AV1Decoder *pbi, ...@@ -2972,16 +2997,6 @@ static int read_uncompressed_header(AV1Decoder *pbi,
#endif #endif
#if CONFIG_AMVR #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) { if (cm->cur_frame_force_integer_mv) {
cm->allow_high_precision_mv = 0; cm->allow_high_precision_mv = 0;
} else { } else {
......
...@@ -3485,6 +3485,26 @@ void write_sequence_header(AV1_COMP *cpi, struct aom_write_bit_buffer *wb) { ...@@ -3485,6 +3485,26 @@ void write_sequence_header(AV1_COMP *cpi, struct aom_write_bit_buffer *wb) {
} }
write_sb_size(seq_params, wb); 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 #endif // CONFIG_REFERENCE_BUFFER || CONFIG_OBU
...@@ -3663,6 +3683,26 @@ static void write_uncompressed_header_frame(AV1_COMP *cpi, ...@@ -3663,6 +3683,26 @@ static void write_uncompressed_header_frame(AV1_COMP *cpi,
write_sequence_header(cpi, wb); write_sequence_header(cpi, wb);
#endif // CONFIG_REFERENCE_BUFFER #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 #if CONFIG_REFERENCE_BUFFER
cm->invalid_delta_frame_id_minus1 = 0; cm->invalid_delta_frame_id_minus1 = 0;
if (cm->seq_params.frame_id_numbers_present_flag) { if (cm->seq_params.frame_id_numbers_present_flag) {
...@@ -3704,20 +3744,9 @@ static void write_uncompressed_header_frame(AV1_COMP *cpi, ...@@ -3704,20 +3744,9 @@ static void write_uncompressed_header_frame(AV1_COMP *cpi,
#else #else
write_frame_size(cm, wb); write_frame_size(cm, wb);
#endif #endif
aom_wb_write_bit(wb, cm->allow_screen_content_tools);
#if CONFIG_INTRABC #if CONFIG_INTRABC
if (cm->allow_screen_content_tools) aom_wb_write_bit(wb, cm->allow_intrabc); if (cm->allow_screen_content_tools) aom_wb_write_bit(wb, cm->allow_intrabc);
#endif // CONFIG_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 { } else {
#if !CONFIG_NO_FRAME_CONTEXT_SIGNALING #if !CONFIG_NO_FRAME_CONTEXT_SIGNALING
if (!cm->error_resilient_mode) { if (!cm->error_resilient_mode) {
...@@ -3750,7 +3779,6 @@ static void write_uncompressed_header_frame(AV1_COMP *cpi, ...@@ -3750,7 +3779,6 @@ static void write_uncompressed_header_frame(AV1_COMP *cpi,
#else #else
write_frame_size(cm, wb); write_frame_size(cm, wb);
#endif #endif
aom_wb_write_bit(wb, cm->allow_screen_content_tools);
#if CONFIG_INTRABC #if CONFIG_INTRABC
if (cm->allow_screen_content_tools) if (cm->allow_screen_content_tools)
aom_wb_write_bit(wb, cm->allow_intrabc); aom_wb_write_bit(wb, cm->allow_intrabc);
...@@ -3798,16 +3826,6 @@ static void write_uncompressed_header_frame(AV1_COMP *cpi, ...@@ -3798,16 +3826,6 @@ static void write_uncompressed_header_frame(AV1_COMP *cpi,
#endif #endif
#if CONFIG_AMVR #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) { if (cm->cur_frame_force_integer_mv) {
cm->allow_high_precision_mv = 0; cm->allow_high_precision_mv = 0;
} else { } else {
...@@ -4017,6 +4035,25 @@ static void write_uncompressed_header_obu(AV1_COMP *cpi, ...@@ -4017,6 +4035,25 @@ static void write_uncompressed_header_obu(AV1_COMP *cpi,
aom_wb_write_bit(wb, cm->show_frame); aom_wb_write_bit(wb, cm->show_frame);
aom_wb_write_bit(wb, cm->error_resilient_mode); 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 #if CONFIG_REFERENCE_BUFFER
cm->invalid_delta_frame_id_minus1 = 0; cm->invalid_delta_frame_id_minus1 = 0;
if (cm->seq_params.frame_id_numbers_present_flag) { if (cm->seq_params.frame_id_numbers_present_flag) {
...@@ -4043,20 +4080,9 @@ static void write_uncompressed_header_obu(AV1_COMP *cpi, ...@@ -4043,20 +4080,9 @@ static void write_uncompressed_header_obu(AV1_COMP *cpi,
#else #else
write_frame_size(cm, wb); write_frame_size(cm, wb);
#endif #endif
aom_wb_write_bit(wb, cm->allow_screen_content_tools);
#if CONFIG_INTRABC #if CONFIG_INTRABC
if (cm->allow_screen_content_tools) aom_wb_write_bit(wb, cm->allow_intrabc); if (cm->allow_screen_content_tools) aom_wb_write_bit(wb, cm->allow_intrabc);
#endif // CONFIG_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) { } else if (cm->frame_type == INTRA_ONLY_FRAME) {
#if !CONFIG_NO_FRAME_CONTEXT_SIGNALING #if !CONFIG_NO_FRAME_CONTEXT_SIGNALING
if (!cm->error_resilient_mode) { if (!cm->error_resilient_mode) {
...@@ -4075,7 +4101,6 @@ static void write_uncompressed_header_obu(AV1_COMP *cpi, ...@@ -4075,7 +4101,6 @@ static void write_uncompressed_header_obu(AV1_COMP *cpi,
#else #else
write_frame_size(cm, wb); write_frame_size(cm, wb);
#endif #endif
aom_wb_write_bit(wb, cm->allow_screen_content_tools);
#if CONFIG_INTRABC #if CONFIG_INTRABC
if (cm->allow_screen_content_tools) if (cm->allow_screen_content_tools)
aom_wb_write_bit(wb, cm->allow_intrabc); aom_wb_write_bit(wb, cm->allow_intrabc);
...@@ -4134,16 +4159,6 @@ static void write_uncompressed_header_obu(AV1_COMP *cpi, ...@@ -4134,16 +4159,6 @@ static void write_uncompressed_header_obu(AV1_COMP *cpi,
#endif #endif
#if CONFIG_AMVR #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) { if (cm->cur_frame_force_integer_mv) {
cm->allow_high_precision_mv = 0; cm->allow_high_precision_mv = 0;
} else { } else {
......
...@@ -4244,12 +4244,17 @@ static void encode_frame_internal(AV1_COMP *cpi) { ...@@ -4244,12 +4244,17 @@ static void encode_frame_internal(AV1_COMP *cpi) {
av1_zero(rdc->comp_pred_diff); av1_zero(rdc->comp_pred_diff);
if (frame_is_intra_only(cm)) { if (frame_is_intra_only(cm)) {
cm->allow_screen_content_tools = if (cm->seq_params.force_screen_content_tools == 2) {
cpi->oxcf.content == AOM_CONTENT_SCREEN || cm->allow_screen_content_tools =
is_screen_content(cpi->source->y_buffer, cpi->oxcf.content == AOM_CONTENT_SCREEN ||
cpi->source->flags & YV12_FLAG_HIGHBITDEPTH, xd->bd, is_screen_content(cpi->source->y_buffer,
cpi->source->y_stride, cpi->source->y_width, cpi->source->flags & YV12_FLAG_HIGHBITDEPTH, xd->bd,
cpi->source->y_height); 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 #if CONFIG_INTRABC
......
...@@ -3261,8 +3261,10 @@ void av1_change_config(struct AV1_COMP *cpi, const AV1EncoderConfig *oxcf) { ...@@ -3261,8 +3261,10 @@ void av1_change_config(struct AV1_COMP *cpi, const AV1EncoderConfig *oxcf) {
cpi->ext_refresh_frame_context_pending = 0; cpi->ext_refresh_frame_context_pending = 0;
highbd_set_var_fns(cpi); highbd_set_var_fns(cpi);
cm->seq_params.force_screen_content_tools = 2;
#if CONFIG_AMVR #if CONFIG_AMVR
cm->seq_force_integer_mv = 2; cm->seq_params.force_integer_mv = 2;
#endif #endif
} }
...@@ -7011,13 +7013,14 @@ int av1_get_compressed_data(AV1_COMP *cpi, unsigned int *frame_flags, ...@@ -7011,13 +7013,14 @@ int av1_get_compressed_data(AV1_COMP *cpi, unsigned int *frame_flags,
#if CONFIG_AMVR #if CONFIG_AMVR
cpi->cur_poc++; cpi->cur_poc++;
if (oxcf->pass != 1 && cpi->common.allow_screen_content_tools) { 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 = struct lookahead_entry *previous_entry =
cpi->lookahead->buf + cpi->previsous_index; cpi->lookahead->buf + cpi->previsous_index;
cpi->common.cur_frame_force_integer_mv = is_integer_mv( cpi->common.cur_frame_force_integer_mv = is_integer_mv(
cpi, cpi->source, &previous_entry->img, cpi->previsou_hash_table); cpi, cpi->source, &previous_entry->img, cpi->previsou_hash_table);
} else { } 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 { } else {
cpi->common.cur_frame_force_integer_mv = 0; cpi->common.cur_frame_force_integer_mv = 0;
......
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