History

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

Jerome Jiang committed 8 years ago

Commit f3a027a1 parent 97e7da45 files · +55 −11Browse files

Unified · Split

5 files
  1. vp8/common/blockd.h +3 −0
  2. vp8/decoder/decodeframe.c +2 −1
  3. vp8/decoder/decoderthreading.h +1 −1
  4. vp8/decoder/onyxd_if.c +7 −1
  5. vp8/decoder/threading.c +42 −8
vp8/common/blockd.h+3−0
@@ -13,6 +13,7 @@ · 12 unmodified lines
1313
1414 void vpx_log(const char *format, ...);
1515
16+#include "vpx/internal/vpx_codec_internal.h"
1617 #include "vpx_config.h"
1718 #include "vpx_scale/yv12config.h"
1819 #include "mv.h"
@@ -288,6 +289,8 @@ typedef struct macroblockd { · 269 unmodified lines
288289
289290 int corrupted;
290291
292+ struct vpx_internal_error_info error_info;
293+
291294 #if ARCH_X86 || ARCH_X86_64
292295 /* This is an intermediate buffer currently used in sub-pixel motion search
293296 * to keep a copy of the reference area. This buffer can be used for other
vp8/decoder/decodeframe.c+2−1
@@ -1220,7 +1220,8 @@ int vp8_decode_frame(VP8D_COMP *pbi) { · 1219 unmodified lines
12201220 if (vpx_atomic_load_acquire(&pbi->b_multithreaded_rd) &&
12211221 pc->multi_token_partition != ONE_PARTITION) {
12221222 unsigned int thread;
1223− vp8mt_decode_mb_rows(pbi, xd);
1223+ if (vp8mt_decode_mb_rows(pbi, xd))
1224+ vpx_internal_error(&pbi->common.error, VPX_CODEC_CORRUPT_FRAME, NULL);
12241225 vp8_yv12_extend_frame_borders(yv12_fb_new);
12251226 for (thread = 0; thread < pbi->decoding_thread_count; ++thread) {
12261227 corrupt_tokens |= pbi->mb_row_di[thread].mbd.corrupted;
vp8/decoder/decoderthreading.h+1−1
@@ -16,7 +16,7 @@ extern "C" { · 15 unmodified lines
1616 #endif
1717
1818 #if CONFIG_MULTITHREAD
19−void vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd);
19+int vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd);
2020 void vp8_decoder_remove_threads(VP8D_COMP *pbi);
2121 void vp8_decoder_create_threads(VP8D_COMP *pbi);
2222 void vp8mt_alloc_temp_buffers(VP8D_COMP *pbi, int width, int prev_mb_rows);
vp8/decoder/onyxd_if.c+7−1
@@ -331,6 +331,7 @@ int vp8dx_receive_compressed_data(VP8D_COMP *pbi, size_t size, · 330 unmodified lines
331331 if (cm->fb_idx_ref_cnt[cm->new_fb_idx] > 0) {
332332 cm->fb_idx_ref_cnt[cm->new_fb_idx]--;
333333 }
334+ pbi->common.error.setjmp = 0;
334335 goto decode_exit;
335336 }
336337
@@ -344,6 +345,12 @@ int vp8dx_receive_compressed_data(VP8D_COMP *pbi, size_t size, · 7 unmodified lines
344345 }
345346
346347 pbi->common.error.error_code = VPX_CODEC_ERROR;
348+ // Propagate the error info.
349+ if (pbi->mb.error_info.error_code != 0) {
350+ pbi->common.error.error_code = pbi->mb.error_info.error_code;
351+ memcpy(pbi->common.error.detail, pbi->mb.error_info.detail,
352+ sizeof(pbi->mb.error_info.detail));
353+ }
347354 goto decode_exit;
348355 }
349356
@@ -382,7 +389,6 @@ int vp8dx_receive_compressed_data(VP8D_COMP *pbi, size_t size, · 32 unmodified lines
382389 pbi->last_time_stamp = time_stamp;
383390
384391 decode_exit:
385− pbi->common.error.setjmp = 0;
386392 vpx_clear_system_state();
387393 return retcode;
388394 }
vp8/decoder/threading.c+42−8
@@ -400,6 +400,21 @@ static void mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd, · 399 unmodified lines
400400 xd->dst.u_buffer = dst_buffer[1] + recon_uvoffset;
401401 xd->dst.v_buffer = dst_buffer[2] + recon_uvoffset;
402402
403+ /* propagate errors from reference frames */
404+ xd->corrupted |= ref_fb_corrupted[xd->mode_info_context->mbmi.ref_frame];
405+
406+ if (xd->corrupted) {
407+ // Move current decoding marcoblock to the end of row for all rows
408+ // assigned to this thread, such that other threads won't be waiting.
409+ for (; mb_row < pc->mb_rows;
410+ mb_row += (pbi->decoding_thread_count + 1)) {
411+ current_mb_col = &pbi->mt_current_mb_col[mb_row];
412+ vpx_atomic_store_release(current_mb_col, pc->mb_cols + nsync);
413+ }
414+ vpx_internal_error(&xd->error_info, VPX_CODEC_CORRUPT_FRAME,
415+ "Corrupted reference frame");
416+ }
417+
403418 xd->pre.y_buffer =
404419 ref_buffer[xd->mode_info_context->mbmi.ref_frame][0] + recon_yoffset;
405420 xd->pre.u_buffer =
@@ -407,9 +422,6 @@ static void mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd, · 1 unmodified line
407422 xd->pre.v_buffer =
408423 ref_buffer[xd->mode_info_context->mbmi.ref_frame][2] + recon_uvoffset;
409424
410− /* propagate errors from reference frames */
411− xd->corrupted |= ref_fb_corrupted[xd->mode_info_context->mbmi.ref_frame];
412−
413425 mt_decode_macroblock(pbi, xd, 0);
414426
415427 xd->left_available = 1;
@@ -557,8 +569,9 @@ static void mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd, · 141 unmodified lines
557569 xd->mode_info_context += xd->mode_info_stride * pbi->decoding_thread_count;
558570 }
559571
560− /* signal end of frame decoding if this thread processed the last mb_row */
561− if (last_mb_row == (pc->mb_rows - 1)) sem_post(&pbi->h_event_end_decoding);
572+ /* signal end of decoding of current thread for current frame */
573+ if (last_mb_row + (int)pbi->decoding_thread_count + 1 >= pc->mb_rows)
574+ sem_post(&pbi->h_event_end_decoding);
562575 }
563576
564577 static THREAD_FUNCTION thread_decoding_proc(void *p_data) {
@@ -576,7 +589,13 @@ static THREAD_FUNCTION thread_decoding_proc(void *p_data) { · 11 unmodified lines
576589 } else {
577590 MACROBLOCKD *xd = &mbrd->mbd;
578591 xd->left_context = &mb_row_left_context;
579−
592+ if (setjmp(xd->error_info.jmp)) {
593+ xd->error_info.setjmp = 0;
594+ // Signal the end of decoding for current thread.
595+ sem_post(&pbi->h_event_end_decoding);
596+ continue;
597+ }
598+ xd->error_info.setjmp = 1;
580599 mt_decode_mb_rows(pbi, xd, ithread + 1);
581600 }
582601 }
@@ -809,7 +828,7 @@ void vp8_decoder_remove_threads(VP8D_COMP *pbi) { · 226 unmodified lines
809828 }
810829 }
811830
812−void vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd) {
831+int vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd) {
813832 VP8_COMMON *pc = &pbi->common;
814833 unsigned int i;
815834 int j;
@@ -855,7 +874,22 @@ void vp8mt_decode_mb_rows(VP8D_COMP *pbi, MACROBLOCKD *xd) { · 39 unmodified lines
855874 sem_post(&pbi->h_event_start_decoding[i]);
856875 }
857876
877+ if (setjmp(xd->error_info.jmp)) {
878+ xd->error_info.setjmp = 0;
879+ xd->corrupted = 1;
880+ // Wait for other threads to finish. This prevents other threads decoding
881+ // the current frame while the main thread starts decoding the next frame,
882+ // which causes a data race.
883+ for (i = 0; i < pbi->decoding_thread_count; ++i)
884+ sem_wait(&pbi->h_event_end_decoding);
885+ return -1;
886+ }
887+
888+ xd->error_info.setjmp = 1;
858889 mt_decode_mb_rows(pbi, xd, 0);
859890
860− sem_wait(&pbi->h_event_end_decoding); /* add back for each frame */
891+ for (i = 0; i < pbi->decoding_thread_count + 1; ++i)
892+ sem_wait(&pbi->h_event_end_decoding); /* add back for each frame */
893+
894+ return 0;
861895 }

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t