From f3a027a46de103c97f9f413fea003dc3d97e2cfc Mon Sep 17 00:00:00 2001 From: Jerome Jiang Date: Tue, 7 Aug 2018 11:10:26 -0700 Subject: [PATCH] vp8: fix to address overflow in decoder. Can't call internal error from the decoder thread. Add vpx_internal_error_info to MACROBLOCKD. When corrupted frame detected, the decoder thread returns to its own context and signal completion of decoding for current frame. The main decoding thread will detect error too and return error code to decoding API call. Each thread will signal end of decoding of the frame. Main thread waits for the signal of all other threads to start decoding next frame. BUG=875626,webm:1496 Change-Id: Icd05fbc558893a4e7d8532c1e7177e7550283a64 --- vp8/common/blockd.h | 3 ++ vp8/decoder/decodeframe.c | 3 +- vp8/decoder/decoderthreading.h | 2 +- vp8/decoder/onyxd_if.c | 8 +++++- vp8/decoder/threading.c | 50 ++++++++++++++++++++++++++++------ 5 files changed, 55 insertions(+), 11 deletions(-) diff --git a/vp8/common/blockd.h b/vp8/common/blockd.h index 22af8980b..cb8de0caf 100644 --- a/vp8/common/blockd.h +++ b/vp8/common/blockd.h @@ -13,6 +13,7 @@ void vpx_log(const char *format, ...); +#include "vpx/internal/vpx_codec_internal.h" #include "vpx_config.h" #include "vpx_scale/yv12config.h" #include "mv.h" @@ -288,6 +289,8 @@ typedef struct macroblockd { int corrupted; + struct vpx_internal_error_info error_info; + #if ARCH_X86 || ARCH_X86_64 /* This is an intermediate buffer currently used in sub-pixel motion search * to keep a copy of the reference area. This buffer can be used for other diff --git a/vp8/decoder/decodeframe.c b/vp8/decoder/decodeframe.c index 82b72d21e..f4842ef03 100644 --- a/vp8/decoder/decodeframe.c +++ b/vp8/decoder/decodeframe.c @@ -1220,7 +1220,8 @@ int vp8_decode_frame(VP8D_COMP *pbi) { if (vpx_atomic_load_acquire(&pbi->b_multithreaded_rd) && pc->multi_token_partition != ONE_PARTITION) { unsigned int thread; - vp8mt_decode_mb_rows(pbi, xd); + if (vp8mt_decode_mb_rows(pbi, xd)) + vpx_internal_error(&pbi->common.error, VPX_CODEC_CORRUPT_FRAME, NULL); vp8_yv12_extend_frame_borders(yv12_fb_new); for (thread = 0; thread < pbi->decoding_thread_count; ++thread) { corrupt_tokens |= pbi->mb_row_di[thread].mbd.corrupted; diff --git a/vp8/decoder/decoderthreading.h b/vp8/decoder/decoderthreading.h index 140295625..3d49bc831 100644 --- a/vp8/decoder/decoderthreading.h +++ b/vp8/decoder/decoderthreading.h @@ -16,7 +16,7 @@ extern "C" { #endif #if CONFIG_MULTITHREAD -void vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd); +int vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd); void vp8_decoder_remove_threads(VP8D_COMP *pbi); void vp8_decoder_create_threads(VP8D_COMP *pbi); void vp8mt_alloc_temp_buffers(VP8D_COMP *pbi, int width, int prev_mb_rows); diff --git a/vp8/decoder/onyxd_if.c b/vp8/decoder/onyxd_if.c index f516eb0c7..2b8dd4ae2 100644 --- a/vp8/decoder/onyxd_if.c +++ b/vp8/decoder/onyxd_if.c @@ -331,6 +331,7 @@ int vp8dx_receive_compressed_data(VP8D_COMP *pbi, size_t size, if (cm->fb_idx_ref_cnt[cm->new_fb_idx] > 0) { cm->fb_idx_ref_cnt[cm->new_fb_idx]--; } + pbi->common.error.setjmp = 0; goto decode_exit; } @@ -344,6 +345,12 @@ int vp8dx_receive_compressed_data(VP8D_COMP *pbi, size_t size, } pbi->common.error.error_code = VPX_CODEC_ERROR; + // Propagate the error info. + if (pbi->mb.error_info.error_code != 0) { + pbi->common.error.error_code = pbi->mb.error_info.error_code; + memcpy(pbi->common.error.detail, pbi->mb.error_info.detail, + sizeof(pbi->mb.error_info.detail)); + } goto decode_exit; } @@ -382,7 +389,6 @@ int vp8dx_receive_compressed_data(VP8D_COMP *pbi, size_t size, pbi->last_time_stamp = time_stamp; decode_exit: - pbi->common.error.setjmp = 0; vpx_clear_system_state(); return retcode; } diff --git a/vp8/decoder/threading.c b/vp8/decoder/threading.c index aadc8dc71..8cc301d4f 100644 --- a/vp8/decoder/threading.c +++ b/vp8/decoder/threading.c @@ -400,6 +400,21 @@ static void mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd, xd->dst.u_buffer = dst_buffer[1] + recon_uvoffset; xd->dst.v_buffer = dst_buffer[2] + recon_uvoffset; + /* propagate errors from reference frames */ + xd->corrupted |= ref_fb_corrupted[xd->mode_info_context->mbmi.ref_frame]; + + if (xd->corrupted) { + // Move current decoding marcoblock to the end of row for all rows + // assigned to this thread, such that other threads won't be waiting. + for (; mb_row < pc->mb_rows; + mb_row += (pbi->decoding_thread_count + 1)) { + current_mb_col = &pbi->mt_current_mb_col[mb_row]; + vpx_atomic_store_release(current_mb_col, pc->mb_cols + nsync); + } + vpx_internal_error(&xd->error_info, VPX_CODEC_CORRUPT_FRAME, + "Corrupted reference frame"); + } + xd->pre.y_buffer = ref_buffer[xd->mode_info_context->mbmi.ref_frame][0] + recon_yoffset; xd->pre.u_buffer = @@ -407,9 +422,6 @@ static void mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd, xd->pre.v_buffer = ref_buffer[xd->mode_info_context->mbmi.ref_frame][2] + recon_uvoffset; - /* propagate errors from reference frames */ - xd->corrupted |= ref_fb_corrupted[xd->mode_info_context->mbmi.ref_frame]; - mt_decode_macroblock(pbi, xd, 0); xd->left_available = 1; @@ -557,8 +569,9 @@ static void mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd, xd->mode_info_context += xd->mode_info_stride * pbi->decoding_thread_count; } - /* signal end of frame decoding if this thread processed the last mb_row */ - if (last_mb_row == (pc->mb_rows - 1)) sem_post(&pbi->h_event_end_decoding); + /* signal end of decoding of current thread for current frame */ + if (last_mb_row + (int)pbi->decoding_thread_count + 1 >= pc->mb_rows) + sem_post(&pbi->h_event_end_decoding); } static THREAD_FUNCTION thread_decoding_proc(void *p_data) { @@ -576,7 +589,13 @@ static THREAD_FUNCTION thread_decoding_proc(void *p_data) { } else { MACROBLOCKD *xd = &mbrd->mbd; xd->left_context = &mb_row_left_context; - + if (setjmp(xd->error_info.jmp)) { + xd->error_info.setjmp = 0; + // Signal the end of decoding for current thread. + sem_post(&pbi->h_event_end_decoding); + continue; + } + xd->error_info.setjmp = 1; mt_decode_mb_rows(pbi, xd, ithread + 1); } } @@ -809,7 +828,7 @@ void vp8_decoder_remove_threads(VP8D_COMP *pbi) { } } -void vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd) { +int vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd) { VP8_COMMON *pc = &pbi->common; unsigned int i; int j; @@ -855,7 +874,22 @@ void vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd) { sem_post(&pbi->h_event_start_decoding[i]); } + if (setjmp(xd->error_info.jmp)) { + xd->error_info.setjmp = 0; + xd->corrupted = 1; + // Wait for other threads to finish. This prevents other threads decoding + // the current frame while the main thread starts decoding the next frame, + // which causes a data race. + for (i = 0; i < pbi->decoding_thread_count; ++i) + sem_wait(&pbi->h_event_end_decoding); + return -1; + } + + xd->error_info.setjmp = 1; mt_decode_mb_rows(pbi, xd, 0); - sem_wait(&pbi->h_event_end_decoding); /* add back for each frame */ + for (i = 0; i < pbi->decoding_thread_count + 1; ++i) + sem_wait(&pbi->h_event_end_decoding); /* add back for each frame */ + + return 0; } -- 2.40.0