| Message ID | b721e88b-c384-e74b-715f-49d748948c7a@carpalis.nl |
|---|---|
| State | Superseded |
| Headers | show |
On Fri, Jun 08, 2018 at 11:01:30AM +0200, Jerome Borsboom wrote: > Change vc1_decode_i_blocks to use vc1_put_blocks_clamped and > ff_vc1_i_loop_filter. > > Signed-off-by: Jerome Borsboom <jerome.borsboom@carpalis.nl> > --- > libavcodec/vc1_block.c | 77 +++++++++++++++++++------------------------------- > 1 file changed, 29 insertions(+), 48 deletions(-) crashes: [vc1 @ 0x11b77200] Slice header damaged ==2065== at 0x120D69C: VALGRIND_PRINTF_BACKTRACE (valgrind.h:4550) ==2065== by 0x120E11C: av_log_default_callback (log.c:351) ==2065== by 0x120E2BB: av_vlog (log.c:377) ==2065== by 0x120E27B: av_log (log.c:369) ==2065== by 0xC3B5B5: vc1_decode_frame (vc1dec.c:1014) ==2065== by 0x8A6BAB: decode_simple_internal (decode.c:398) ==2065== by 0x8A7832: decode_simple_receive_frame (decode.c:594) ==2065== by 0x8A78FD: decode_receive_frame_internal (decode.c:612) ==2065== by 0x8A7B75: avcodec_send_packet (decode.c:674) ==2065== by 0x43373E: decode (ffmpeg.c:2238) ==2065== by 0x433F98: decode_video (ffmpeg.c:2382) ==2065== by 0x434FE0: process_input_packet (ffmpeg.c:2623) ==2065== by 0x43C207: process_input (ffmpeg.c:4461) ==2065== by 0x43C7B0: transcode_step (ffmpeg.c:4581) ==2065== by 0x43C92C: transcode (ffmpeg.c:4635) ==2065== by 0x43D199: main (ffmpeg.c:4842) ==2065== Invalid read of size 8 ==2065== at 0xD790BD: ??? (libavcodec/x86/vc1dsp_loopfilter.asm:302) ==2065== by 0xC26AA6: vc1_i_v_loop_filter (vc1_loopfilter.c:239) ==2065== by 0xC26BFB: ff_vc1_i_loop_filter (vc1_loopfilter.c:266) ==2065== by 0xC23C72: vc1_decode_i_blocks (vc1_block.c:2594) ==2065== by 0xC259D4: ff_vc1_decode_blocks (vc1_block.c:2969) ==2065== by 0xC3B7D6: vc1_decode_frame (vc1dec.c:1042) ==2065== by 0x8A6BAB: decode_simple_internal (decode.c:398) ==2065== by 0x8A7832: decode_simple_receive_frame (decode.c:594) ==2065== by 0x8A78FD: decode_receive_frame_internal (decode.c:612) ==2065== by 0x8A7B75: avcodec_send_packet (decode.c:674) ==2065== by 0x43373E: decode (ffmpeg.c:2238) ==2065== by 0x433F98: decode_video (ffmpeg.c:2382) ==2065== by 0x434FE0: process_input_packet (ffmpeg.c:2623) ==2065== by 0x43C207: process_input (ffmpeg.c:4461) ==2065== by 0x43C7B0: transcode_step (ffmpeg.c:4581) ==2065== by 0x43C92C: transcode (ffmpeg.c:4635) ==2065== by 0x43D199: main (ffmpeg.c:4842) ==2065== Address 0x11f25100 is not stack'd, malloc'd or (recently) free'd .... ==2065== Process terminating with default action of signal 11 (SIGSEGV) ==2065== General Protection Fault ==2065== at 0x13786A6: ??? (in ffmpeg/ffmpeg_g) ==2065== by 0x11F8D78: av_buffer_pool_uninit (buffer.c:285) ==2065== by 0xBF5F7B: avcodec_close (utils.c:1089) ==2065== by 0x43CCB5: transcode (ffmpeg.c:4697) ==2065== by 0x43D199: main (ffmpeg.c:4842) [...]
Thank you for the rigorous testing of my patches. I try to be careful when changing things, but every now and then I unwittingly may break things that do not surface in setup. There is something strange with the backtraces. The 'Slice header damaged' error indicates that the source file is coded with slices. Slices, however, only occur in VC-1 advanced profile. The backtrace with the invalid read contains vc1_decode_i_blocks which is only used in Simple and Main profile. I currently do not see how this adds up. Is this crash related to a specific source file? If so, could you share it? Regards, Jerome > > crashes: > > [vc1 @ 0x11b77200] Slice header damaged > ==2065== at 0x120D69C: VALGRIND_PRINTF_BACKTRACE (valgrind.h:4550) > ==2065== by 0x120E11C: av_log_default_callback (log.c:351) > ==2065== by 0x120E2BB: av_vlog (log.c:377) > ==2065== by 0x120E27B: av_log (log.c:369) > ==2065== by 0xC3B5B5: vc1_decode_frame (vc1dec.c:1014) > ==2065== by 0x8A6BAB: decode_simple_internal (decode.c:398) > ==2065== by 0x8A7832: decode_simple_receive_frame (decode.c:594) > ==2065== by 0x8A78FD: decode_receive_frame_internal (decode.c:612) > ==2065== by 0x8A7B75: avcodec_send_packet (decode.c:674) > ==2065== by 0x43373E: decode (ffmpeg.c:2238) > ==2065== by 0x433F98: decode_video (ffmpeg.c:2382) > ==2065== by 0x434FE0: process_input_packet (ffmpeg.c:2623) > ==2065== by 0x43C207: process_input (ffmpeg.c:4461) > ==2065== by 0x43C7B0: transcode_step (ffmpeg.c:4581) > ==2065== by 0x43C92C: transcode (ffmpeg.c:4635) > ==2065== by 0x43D199: main (ffmpeg.c:4842) > ==2065== Invalid read of size 8 > ==2065== at 0xD790BD: ??? (libavcodec/x86/vc1dsp_loopfilter.asm:302) > ==2065== by 0xC26AA6: vc1_i_v_loop_filter (vc1_loopfilter.c:239) > ==2065== by 0xC26BFB: ff_vc1_i_loop_filter (vc1_loopfilter.c:266) > ==2065== by 0xC23C72: vc1_decode_i_blocks (vc1_block.c:2594) > ==2065== by 0xC259D4: ff_vc1_decode_blocks (vc1_block.c:2969) > ==2065== by 0xC3B7D6: vc1_decode_frame (vc1dec.c:1042) > ==2065== by 0x8A6BAB: decode_simple_internal (decode.c:398) > ==2065== by 0x8A7832: decode_simple_receive_frame (decode.c:594) > ==2065== by 0x8A78FD: decode_receive_frame_internal (decode.c:612) > ==2065== by 0x8A7B75: avcodec_send_packet (decode.c:674) > ==2065== by 0x43373E: decode (ffmpeg.c:2238) > ==2065== by 0x433F98: decode_video (ffmpeg.c:2382) > ==2065== by 0x434FE0: process_input_packet (ffmpeg.c:2623) > ==2065== by 0x43C207: process_input (ffmpeg.c:4461) > ==2065== by 0x43C7B0: transcode_step (ffmpeg.c:4581) > ==2065== by 0x43C92C: transcode (ffmpeg.c:4635) > ==2065== by 0x43D199: main (ffmpeg.c:4842) > ==2065== Address 0x11f25100 is not stack'd, malloc'd or (recently) free'd > > .... > > ==2065== Process terminating with default action of signal 11 (SIGSEGV) > ==2065== General Protection Fault > ==2065== at 0x13786A6: ??? (in ffmpeg/ffmpeg_g) > ==2065== by 0x11F8D78: av_buffer_pool_uninit (buffer.c:285) > ==2065== by 0xBF5F7B: avcodec_close (utils.c:1089) > ==2065== by 0x43CCB5: transcode (ffmpeg.c:4697) > ==2065== by 0x43D199: main (ffmpeg.c:4842) > > [...]
On Sun, Jun 10, 2018 at 10:18:45AM +0200, Jerome Borsboom wrote: > Thank you for the rigorous testing of my patches. I try to be careful > when changing things, but every now and then I unwittingly may break > things that do not surface in setup. > > There is something strange with the backtraces. The 'Slice header > damaged' error indicates that the source file is coded with slices. > Slices, however, only occur in VC-1 advanced profile. The backtrace with > the invalid read contains vc1_decode_i_blocks which is only used in > Simple and Main profile. I currently do not see how this adds up. > > Is this crash related to a specific source file? If so, could you share it? will mail it to you privatly thx [...]
diff --git a/libavcodec/vc1_block.c b/libavcodec/vc1_block.c index 1dc8c6422d..a7ba261ccb 100644 --- a/libavcodec/vc1_block.c +++ b/libavcodec/vc1_block.c @@ -2541,26 +2541,24 @@ static void vc1_decode_i_blocks(VC1Context *v) s->mb_x = 0; init_block_index(v); for (; s->mb_x < v->end_mb_x; s->mb_x++) { - uint8_t *dst[6]; + int16_t (*block)[64] = v->block[v->cur_blk_idx]; ff_update_block_index(s); - dst[0] = s->dest[0]; - dst[1] = dst[0] + 8; - dst[2] = s->dest[0] + s->linesize * 8; - dst[3] = dst[2] + 8; - dst[4] = s->dest[1]; - dst[5] = s->dest[2]; - s->bdsp.clear_blocks(s->block[0]); + s->bdsp.clear_blocks(block[0]); mb_pos = s->mb_x + s->mb_y * s->mb_width; s->current_picture.mb_type[mb_pos] = MB_TYPE_INTRA; s->current_picture.qscale_table[mb_pos] = v->pq; - s->current_picture.motion_val[1][s->block_index[0]][0] = 0; - s->current_picture.motion_val[1][s->block_index[0]][1] = 0; + for (int i = 0; i < 4; i++) { + s->current_picture.motion_val[1][s->block_index[i]][0] = 0; + s->current_picture.motion_val[1][s->block_index[i]][1] = 0; + } // do actual MB decoding and displaying cbp = get_vlc2(&v->s.gb, ff_msmp4_mb_i_vlc.table, MB_INTRA_VLC_BITS, 2); v->s.ac_pred = get_bits1(&v->s.gb); for (k = 0; k < 6; k++) { + v->mb_type[0][s->block_index[k]] = 1; + val = ((cbp >> (5 - k)) & 1); if (k < 4) { @@ -2570,52 +2568,30 @@ static void vc1_decode_i_blocks(VC1Context *v) } cbp |= val << (5 - k); - vc1_decode_i_block(v, s->block[k], k, val, (k < 4) ? v->codingset : v->codingset2); + vc1_decode_i_block(v, block[k], k, val, (k < 4) ? v->codingset : v->codingset2); if (CONFIG_GRAY && k > 3 && (s->avctx->flags & AV_CODEC_FLAG_GRAY)) continue; - v->vc1dsp.vc1_inv_trans_8x8(s->block[k]); - if (v->pq >= 9 && v->overlap) { - if (v->rangeredfrm) + v->vc1dsp.vc1_inv_trans_8x8(block[k]); + } + + if (v->overlap && v->pq >= 9) { + ff_vc1_i_overlap_filter(v); + if (v->rangeredfrm) + for (k = 0; k < 6; k++) for (j = 0; j < 64; j++) - s->block[k][j] <<= 1; - s->idsp.put_signed_pixels_clamped(s->block[k], dst[k], - k & 4 ? s->uvlinesize - : s->linesize); - } else { - if (v->rangeredfrm) + block[k][j] <<= 1; + vc1_put_blocks_clamped(v, 1); + } else { + if (v->rangeredfrm) + for (k = 0; k < 6; k++) for (j = 0; j < 64; j++) - s->block[k][j] = (s->block[k][j] - 64) << 1; - s->idsp.put_pixels_clamped(s->block[k], dst[k], - k & 4 ? s->uvlinesize - : s->linesize); - } + block[k][j] = (block[k][j] - 64) << 1; + vc1_put_blocks_clamped(v, 0); } - if (v->pq >= 9 && v->overlap) { - if (s->mb_x) { - v->vc1dsp.vc1_h_overlap(s->dest[0], s->linesize); - v->vc1dsp.vc1_h_overlap(s->dest[0] + 8 * s->linesize, s->linesize); - if (!CONFIG_GRAY || !(s->avctx->flags & AV_CODEC_FLAG_GRAY)) { - v->vc1dsp.vc1_h_overlap(s->dest[1], s->uvlinesize); - v->vc1dsp.vc1_h_overlap(s->dest[2], s->uvlinesize); - } - } - v->vc1dsp.vc1_h_overlap(s->dest[0] + 8, s->linesize); - v->vc1dsp.vc1_h_overlap(s->dest[0] + 8 * s->linesize + 8, s->linesize); - if (!s->first_slice_line) { - v->vc1dsp.vc1_v_overlap(s->dest[0], s->linesize); - v->vc1dsp.vc1_v_overlap(s->dest[0] + 8, s->linesize); - if (!CONFIG_GRAY || !(s->avctx->flags & AV_CODEC_FLAG_GRAY)) { - v->vc1dsp.vc1_v_overlap(s->dest[1], s->uvlinesize); - v->vc1dsp.vc1_v_overlap(s->dest[2], s->uvlinesize); - } - } - v->vc1dsp.vc1_v_overlap(s->dest[0] + 8 * s->linesize, s->linesize); - v->vc1dsp.vc1_v_overlap(s->dest[0] + 8 * s->linesize + 8, s->linesize); - } if (v->s.loop_filter) - ff_vc1_loop_filter_iblk(v, v->pq); + ff_vc1_i_loop_filter(v); if (get_bits_count(&s->gb) > v->bits) { ff_er_add_slice(&s->er, 0, 0, s->mb_x, s->mb_y, ER_MB_ERROR); @@ -2623,6 +2599,11 @@ static void vc1_decode_i_blocks(VC1Context *v) get_bits_count(&s->gb), v->bits); return; } + + v->topleft_blk_idx = (v->topleft_blk_idx + 1) % (v->end_mb_x + 2); + v->top_blk_idx = (v->top_blk_idx + 1) % (v->end_mb_x + 2); + v->left_blk_idx = (v->left_blk_idx + 1) % (v->end_mb_x + 2); + v->cur_blk_idx = (v->cur_blk_idx + 1) % (v->end_mb_x + 2); } if (!v->s.loop_filter) ff_mpeg_draw_horiz_band(s, s->mb_y * 16, 16);
Change vc1_decode_i_blocks to use vc1_put_blocks_clamped and ff_vc1_i_loop_filter. Signed-off-by: Jerome Borsboom <jerome.borsboom@carpalis.nl> --- libavcodec/vc1_block.c | 77 +++++++++++++++++++------------------------------- 1 file changed, 29 insertions(+), 48 deletions(-)