Do not acquire extra fb references.
The cm->ref_frame_map and cm->next_ref_frame_map arrays hold references
to frame buffers. The policy is that whenever we store a frame buffer
index in cm->ref_frame_map or cm->next_ref_frame_map, we increase the
ref_count of that frame buffer. Conversely, whenever we overwrite an
element of these arrays with another value, we need to decrease the
ref_count of the old frame buffer.
Under this policy, generate_next_ref_frame_map() acquires an extra frame
buffer reference in the "if (mask & 1)" case. This still works out OK
because both release_frame_buffers() and swap_frame_buffers() compensate
for the extra extra frame buffer references, but it makes the code very
confusing.
This CL fixes generate_next_ref_frame_map() so that it does not acquire
the extra frame buffer references, and makes the corresponding changes
to release_frame_buffers() and swap_frame_buffers().
BUG=aomedia:2266
Change-Id: I88fb9c06f17fb5d3ec77ca40c063f763af88e9de
diff --git a/av1/decoder/decodeframe.c b/av1/decoder/decodeframe.c
index 95820e5..a30b267 100644
--- a/av1/decoder/decodeframe.c
+++ b/av1/decoder/decodeframe.c
@@ -4681,26 +4681,25 @@
RefCntBuffer *const frame_bufs = pool->frame_bufs;
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 = pbi->refresh_frame_flags; mask; mask >>= 1) {
if (mask & 1) {
cm->next_ref_frame_map[ref_index] = cm->new_fb_idx;
- ++cm->cur_frame->ref_count;
} else {
cm->next_ref_frame_map[ref_index] = cm->ref_frame_map[ref_index];
}
- // Current thread holds the reference frame.
- if (cm->ref_frame_map[ref_index] >= 0)
- ++frame_bufs[cm->ref_frame_map[ref_index]].ref_count;
+ if (cm->next_ref_frame_map[ref_index] >= 0)
+ ++frame_bufs[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];
-
- // Current thread holds the reference frame.
- if (cm->ref_frame_map[ref_index] >= 0)
- ++frame_bufs[cm->ref_frame_map[ref_index]].ref_count;
+ if (cm->next_ref_frame_map[ref_index] >= 0)
+ ++frame_bufs[cm->next_ref_frame_map[ref_index]].ref_count;
}
unlock_buffer_pool(pool);
pbi->hold_ref_buf = 1;
diff --git a/av1/decoder/decoder.c b/av1/decoder/decoder.c
index 019db7e..773305d 100644
--- a/av1/decoder/decoder.c
+++ b/av1/decoder/decoder.c
@@ -324,27 +324,13 @@
RefCntBuffer *const frame_bufs = cm->buffer_pool->frame_bufs;
lock_buffer_pool(pool);
- // Release all the reference buffers if worker thread is holding them.
+ // Release all the reference buffers in cm->next_ref_frame_map if the worker
+ // thread is holding them.
if (pbi->hold_ref_buf) {
- int ref_index = 0, mask;
- for (mask = pbi->refresh_frame_flags; mask; mask >>= 1) {
- const int old_idx = cm->ref_frame_map[ref_index];
- // Current thread releases the holding of reference frame.
- decrease_ref_count(old_idx, frame_bufs, pool);
-
- // Release the reference frame holding in the reference map for the
- // decoding of the next frame.
- if (mask & 1) {
- const int new_idx = cm->next_ref_frame_map[ref_index];
- decrease_ref_count(new_idx, frame_bufs, pool);
- }
- ++ref_index;
- }
-
- // Current thread releases the holding of reference frame.
- for (; ref_index < REF_FRAMES; ++ref_index) {
- const int old_idx = cm->ref_frame_map[ref_index];
- decrease_ref_count(old_idx, frame_bufs, pool);
+ int ref_index;
+ for (ref_index = 0; ref_index < REF_FRAMES; ++ref_index) {
+ const int new_idx = cm->next_ref_frame_map[ref_index];
+ decrease_ref_count(new_idx, frame_bufs, pool);
}
pbi->hold_ref_buf = 0;
}
@@ -376,19 +362,16 @@
assert(IMPLIES(!pbi->hold_ref_buf,
cm->show_existing_frame && !cm->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].
for (mask = pbi->refresh_frame_flags; mask; mask >>= 1) {
const int old_idx = cm->ref_frame_map[ref_index];
- // Current thread releases the holding of reference frame.
decrease_ref_count(old_idx, frame_bufs, pool);
-
- // Release the reference frame holding in the reference map for the
- // decoding of the next frame.
- if (mask & 1) decrease_ref_count(old_idx, frame_bufs, pool);
cm->ref_frame_map[ref_index] = cm->next_ref_frame_map[ref_index];
++ref_index;
}
- // Current thread releases the holding of reference frame.
const int check_on_show_existing_frame =
!cm->show_existing_frame || cm->reset_decoder_state;
for (; ref_index < REF_FRAMES && check_on_show_existing_frame;
