Remove cm->next_ref_frame_map, pbi->hold_ref_buf.
cm->next_ref_frame_map was intended to support frame parallel decoding.
Since frame parallel decoding was not implemented, we can remove
cm->next_ref_frame_map and the associated pbi->hold_ref_buf boolean.
swap_frame_buffers() is renamed update_frame_buffers() because the
function no longer swaps cm->ref_frame_map and cm->next_ref_frame_map;
now it updates cm->ref_frame_map. Also, release_frame_buffers() is
renamed release_current_frame() because the only frame buffer it
releases now is cm->current_frame.
TESTED=test_libaom --gtest_filter=*InvalidFileTest*
BUG=aomedia:2110,oss-fuzz:16437
Change-Id: I32dab3d01fb4574f2563f535c24804c6775933d2
diff --git a/av1/common/onyxc_int.h b/av1/common/onyxc_int.h
index 38b76cd..d2abfd7 100644
--- a/av1/common/onyxc_int.h
+++ b/av1/common/onyxc_int.h
@@ -115,7 +115,6 @@
// - cm->ref_frame_map[]
// - cm->cur_frame
// - cm->scaled_ref_buf[] (encoder only)
- // - cm->next_ref_frame_map[] (decoder only)
// - pbi->output_frame_index[] (decoder only)
// With that definition, 'ref_count' is the number of reference-holding
// variables that are currently referencing this buffer.
@@ -362,9 +361,6 @@
// a pointer to the buffer in the buffer pool 'cm->buffer_pool.frame_bufs'.
RefCntBuffer *ref_frame_map[REF_FRAMES];
- // Prepare ref_frame_map for the next frame.
- // Only used in frame parallel decode.
- RefCntBuffer *next_ref_frame_map[REF_FRAMES];
FRAME_TYPE last_frame_type; /* last frame's frame type for motion search.*/
int show_frame;
diff --git a/av1/decoder/decodeframe.c b/av1/decoder/decodeframe.c
index b51a8e3..f355ca9 100644
--- a/av1/decoder/decodeframe.c
+++ b/av1/decoder/decodeframe.c
@@ -4540,36 +4540,6 @@
}
}
-// Generate next_ref_frame_map.
-static void generate_next_ref_frame_map(AV1Decoder *const pbi) {
- AV1_COMMON *const cm = &pbi->common;
- BufferPool *const pool = cm->buffer_pool;
-
- lock_buffer_pool(pool);
- // cm->next_ref_frame_map holds references to frame buffers. After storing a
- // frame buffer index in cm->next_ref_frame_map, we need to increase the
- // frame buffer's ref_count.
- int ref_index = 0;
- for (int mask = cm->current_frame.refresh_frame_flags; mask; mask >>= 1) {
- if (mask & 1) {
- cm->next_ref_frame_map[ref_index] = cm->cur_frame;
- } else {
- cm->next_ref_frame_map[ref_index] = cm->ref_frame_map[ref_index];
- }
- if (cm->next_ref_frame_map[ref_index] != NULL)
- ++cm->next_ref_frame_map[ref_index]->ref_count;
- ++ref_index;
- }
-
- for (; ref_index < REF_FRAMES; ++ref_index) {
- cm->next_ref_frame_map[ref_index] = cm->ref_frame_map[ref_index];
- if (cm->next_ref_frame_map[ref_index] != NULL)
- ++cm->next_ref_frame_map[ref_index]->ref_count;
- }
- unlock_buffer_pool(pool);
- pbi->hold_ref_buf = 1;
-}
-
// If the refresh_frame_flags bitmask is set, update reference frame id values
// and mark frames as valid for reference.
static void update_ref_frame_id(AV1_COMMON *const cm, int frame_id) {
@@ -4607,23 +4577,12 @@
update_ref_frame_id(cm, cm->current_frame_id);
cm->refresh_frame_context = REFRESH_FRAME_CONTEXT_DISABLED;
-
- generate_next_ref_frame_map(pbi);
-
- // Reload the adapted CDFs from when we originally coded this keyframe
- *cm->fc = cm->next_ref_frame_map[existing_frame_idx]->frame_context;
}
static INLINE void reset_frame_buffers(AV1_COMMON *cm) {
RefCntBuffer *const frame_bufs = cm->buffer_pool->frame_bufs;
int i;
- // We have not stored any references to frame buffers in
- // cm->next_ref_frame_map, so we can directly reset it to all NULL.
- for (i = 0; i < REF_FRAMES; ++i) {
- cm->next_ref_frame_map[i] = NULL;
- }
-
lock_buffer_pool(cm->buffer_pool);
reset_ref_frame_map(cm);
assert(cm->cur_frame->ref_count == 1);
@@ -4703,9 +4662,8 @@
lock_buffer_pool(pool);
assert(frame_to_show->ref_count > 0);
// cm->cur_frame should be the buffer referenced by the return value
- // of the get_free_fb() call in av1_receive_compressed_data(), and
- // generate_next_ref_frame_map() has not been called, so ref_count
- // should still be 1.
+ // of the get_free_fb() call in assign_cur_frame_new_fb() (called by
+ // av1_receive_compressed_data()), so the ref_count should be 1.
assert(cm->cur_frame->ref_count == 1);
// assign_frame_buffer_p() decrements ref_count directly rather than
// call decrease_ref_count(). If cm->cur_frame->raw_frame_buffer has
@@ -5135,8 +5093,6 @@
" state");
}
- generate_next_ref_frame_map(pbi);
-
if (cm->allow_intrabc) {
// Set parameters corresponding to no filtering.
struct loopfilter *lf = &cm->lf;
diff --git a/av1/decoder/decoder.c b/av1/decoder/decoder.c
index 4cef5e6..ecd267e 100644
--- a/av1/decoder/decoder.c
+++ b/av1/decoder/decoder.c
@@ -138,7 +138,6 @@
// Initialize the references to not point to any frame buffers.
for (int i = 0; i < REF_FRAMES; i++) {
cm->ref_frame_map[i] = NULL;
- cm->next_ref_frame_map[i] = NULL;
}
cm->current_frame.frame_number = 0;
@@ -356,22 +355,12 @@
return cm->error.error_code;
}
-static void release_frame_buffers(AV1Decoder *pbi) {
+static void release_current_frame(AV1Decoder *pbi) {
AV1_COMMON *const cm = &pbi->common;
BufferPool *const pool = cm->buffer_pool;
cm->cur_frame->buf.corrupted = 1;
lock_buffer_pool(pool);
- // Release all the reference buffers in cm->next_ref_frame_map if the worker
- // thread is holding them.
- if (pbi->hold_ref_buf) {
- for (int ref_index = 0; ref_index < REF_FRAMES; ++ref_index) {
- decrease_ref_count(cm->next_ref_frame_map[ref_index], pool);
- cm->next_ref_frame_map[ref_index] = NULL;
- }
- pbi->hold_ref_buf = 0;
- }
- // Release current frame.
decrease_ref_count(cm->cur_frame, pool);
unlock_buffer_pool(pool);
cm->cur_frame = NULL;
@@ -382,7 +371,7 @@
//
// This functions returns void. It reports failure by setting
// cm->error.error_code.
-static void swap_frame_buffers(AV1Decoder *pbi, int frame_decoded) {
+static void update_frame_buffers(AV1Decoder *pbi, int frame_decoded) {
int ref_index = 0, mask;
AV1_COMMON *const cm = &pbi->common;
BufferPool *const pool = cm->buffer_pool;
@@ -391,33 +380,19 @@
lock_buffer_pool(pool);
// In ext-tile decoding, the camera frame header is only decoded once. So,
- // we don't release the references here.
+ // we don't update the references here.
if (!pbi->camera_frame_header_ready) {
- // If we are not holding reference buffers in cm->next_ref_frame_map,
- // assert that the following two for loops are no-ops.
- assert(IMPLIES(!pbi->hold_ref_buf,
- cm->current_frame.refresh_frame_flags == 0));
- assert(IMPLIES(!pbi->hold_ref_buf,
- cm->show_existing_frame && !pbi->reset_decoder_state));
-
- // The following two for loops need to release the reference stored in
- // cm->ref_frame_map[ref_index] before transferring the reference stored
- // in cm->next_ref_frame_map[ref_index] to cm->ref_frame_map[ref_index].
+ // The following for loop needs to release the reference stored in
+ // cm->ref_frame_map[ref_index] before storing a reference to
+ // cm->cur_frame in cm->ref_frame_map[ref_index].
for (mask = cm->current_frame.refresh_frame_flags; mask; mask >>= 1) {
- decrease_ref_count(cm->ref_frame_map[ref_index], pool);
- cm->ref_frame_map[ref_index] = cm->next_ref_frame_map[ref_index];
- cm->next_ref_frame_map[ref_index] = NULL;
+ if (mask & 1) {
+ decrease_ref_count(cm->ref_frame_map[ref_index], pool);
+ cm->ref_frame_map[ref_index] = cm->cur_frame;
+ ++cm->cur_frame->ref_count;
+ }
++ref_index;
}
-
- const int check_on_show_existing_frame =
- !cm->show_existing_frame || pbi->reset_decoder_state;
- for (; ref_index < REF_FRAMES && check_on_show_existing_frame;
- ++ref_index) {
- decrease_ref_count(cm->ref_frame_map[ref_index], pool);
- cm->ref_frame_map[ref_index] = cm->next_ref_frame_map[ref_index];
- cm->next_ref_frame_map[ref_index] = NULL;
- }
}
if (cm->show_existing_frame || cm->show_frame) {
@@ -448,10 +423,6 @@
unlock_buffer_pool(pool);
} else {
- // The code here assumes we are not holding reference buffers in
- // cm->next_ref_frame_map. If this assertion fails, we are leaking the
- // frame buffer references in cm->next_ref_frame_map.
- assert(IMPLIES(!pbi->camera_frame_header_ready, !pbi->hold_ref_buf));
// Nothing was decoded, so just drop this frame buffer
lock_buffer_pool(pool);
decrease_ref_count(cm->cur_frame, pool);
@@ -460,8 +431,6 @@
cm->cur_frame = NULL;
if (!pbi->camera_frame_header_ready) {
- pbi->hold_ref_buf = 0;
-
// Invalidate these references until the next frame starts.
for (ref_index = 0; ref_index < INTER_REFS_PER_FRAME; ref_index++) {
cm->remapped_ref_idx[ref_index] = INVALID_IDX;
@@ -494,8 +463,6 @@
return 1;
}
- if (!pbi->camera_frame_header_ready) pbi->hold_ref_buf = 0;
-
// The jmp_buf is valid only for the duration of the function that calls
// setjmp(). Therefore, this function must reset the 'setjmp' field to 0
// before it returns.
@@ -512,7 +479,7 @@
winterface->sync(&pbi->tile_workers[i]);
}
- release_frame_buffers(pbi);
+ release_current_frame(pbi);
aom_clear_system_state();
return -1;
}
@@ -524,7 +491,7 @@
if (frame_decoded < 0) {
assert(cm->error.error_code != AOM_CODEC_OK);
- release_frame_buffers(pbi);
+ release_current_frame(pbi);
cm->error.setjmp = 0;
return 1;
}
@@ -539,8 +506,8 @@
#endif
// Note: At this point, this function holds a reference to cm->cur_frame
- // in the buffer pool. This reference is consumed by swap_frame_buffers().
- swap_frame_buffers(pbi, frame_decoded);
+ // in the buffer pool. This reference is consumed by update_frame_buffers().
+ update_frame_buffers(pbi, frame_decoded);
if (frame_decoded) {
pbi->decoding_first_frame = 0;
diff --git a/av1/decoder/decoder.h b/av1/decoder/decoder.h
index b70630f..140ff4f 100644
--- a/av1/decoder/decoder.h
+++ b/av1/decoder/decoder.h
@@ -197,9 +197,7 @@
int allow_lowbitdepth;
int max_threads;
int inv_tile_order;
- int need_resync; // wait for key/intra-only frame.
- int hold_ref_buf; // Boolean: whether we are holding reference buffers in
- // common.next_ref_frame_map.
+ int need_resync; // wait for key/intra-only frame.
int reset_decoder_state;
int tile_size_bytes;
diff --git a/test/invalid_file_test.cc b/test/invalid_file_test.cc
index 11b8ff3..1f10a2b 100644
--- a/test/invalid_file_test.cc
+++ b/test/invalid_file_test.cc
@@ -144,6 +144,7 @@
{ 1, "invalid-oss-fuzz-11479.ivf", "invalid-oss-fuzz-11479.ivf.res.2" },
{ 1, "invalid-oss-fuzz-11523.ivf", "invalid-oss-fuzz-11523.ivf.res.2" },
{ 4, "invalid-oss-fuzz-15363.ivf", NULL },
+ { 1, "invalid-oss-fuzz-16437.ivf", NULL },
};
AV1_INSTANTIATE_TEST_CASE(InvalidFileTest,
diff --git a/test/test-data.sha1 b/test/test-data.sha1
index b2c8d6e..d162885 100644
--- a/test/test-data.sha1
+++ b/test/test-data.sha1
@@ -30,6 +30,8 @@
3198c7af55a7d50173ce3c369c0cf2d9cdfface6 *invalid-oss-fuzz-11523.ivf.res.2
cb445173be760c3554f1740ce4d119f57a7be043 *invalid-oss-fuzz-15363.ivf
d3964f9dad9f60363c81b688324d95b4ec7c8038 *invalid-oss-fuzz-15363.ivf.res
+5b697360bf0f02de31bae9b8da78e93570958fa4 *invalid-oss-fuzz-16437.ivf
+09d2af8dd22201dd8d48e5dcfcaed281ff9422c7 *invalid-oss-fuzz-16437.ivf.res
ccbe4081557eb44820a0e6337c4a094421826b9a *invalid-oss-fuzz-9288.ivf
67c54283fe1a26ccf02cc991e4f9a1eea3ac5e78 *invalid-oss-fuzz-9288.ivf.res
c0960f032484579f967881cc025b71cfd7a79ee1 *invalid-oss-fuzz-9463.ivf
diff --git a/test/test_data_util.cmake b/test/test_data_util.cmake
index 79aa88f..3206f59 100644
--- a/test/test_data_util.cmake
+++ b/test/test_data_util.cmake
@@ -547,6 +547,8 @@
"invalid-oss-fuzz-11523.ivf.res.2"
"invalid-oss-fuzz-15363.ivf"
"invalid-oss-fuzz-15363.ivf.res"
+ "invalid-oss-fuzz-16437.ivf"
+ "invalid-oss-fuzz-16437.ivf.res"
"invalid-oss-fuzz-9288.ivf"
"invalid-oss-fuzz-9288.ivf.res"
"invalid-oss-fuzz-9463.ivf"
