[Mesa-dev] [PATCH 5/6] vl: Remove most members of pipe_video_decoder

[Maarten Lankhorst]

Hey Andy,

On 12/06/2011 10:54 PM, Andy Furniss wrote:
> Maarten Lankhorst wrote:
>> create_buffer, destroy_buffer and set_buffer are a curiosity of vl_mpeg12_decoder
>> and shouldn't be part of the api.
>>
>> set_quant_matrix and set_reference_frames shouldn't be separate calls, but part of
>> picparm, which is a requirement for h264 to work.
>> set_decode_target and set_picture_parameters should instead be passed as argument to
>> decode_(bitstream,macroblocks). flush is used to signal in XvMC that current frame has
>> ended. begin_frame and end_frame are moved into vl_mpeg12_decoder internally.
>
> I get a crash with R600 and mplayer -vc ffmpeg12vdpau after this patch.
>
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0xb6c946d0 (LWP 31981)]
> 0xb6733579 in vl_mpeg12_decode_macroblock (decoder=0xad83018, target=0x0, picture=0x0, macroblocks=0xbfc6edb4, num_macroblocks=1) at vl/vl_mpeg12_decoder.c:644
> 644              buf->mv_stream[i][mb_addr] = MotionVectorToPipe
> (gdb) bt
> #0  0xb6733579 in vl_mpeg12_decode_macroblock (decoder=0xad83018, target=0x0, picture=0x0, macroblocks=0xbfc6edb4, num_macroblocks=1) at vl/vl_mpeg12_decoder.c:644
> #1  0xb6734e8e in vl_mpg12_bs_decode (bs=0xb7b13c0, n=<value optimized out>, len=40001, lens=0xbfc6ee60, buffer=0xbfc6ee80) at vl/vl_mpeg12_bitstream.c:930
> #2  0xb673314b in vl_mpeg12_decode_bitstream (decoder=0xad83018, target=0xb81e470, picture=0xbfc6eec8, n=1, total_len=40001, lens=0xbfc6ee60, data=0xbfc6ee80) at vl/vl_mpeg12_decoder.c:707
> #3  0xb66d2d74 in vlVdpDecoderRender (decoder=5, target=14, picture_info=0x8900728, bitstream_buffer_count=1, bitstream_buffers=0xb81da18) at decode.c:382
> #4  0x080f053f in draw_slice (image=0xbfc6ef9c, stride=0xbfc6ef8c, w=720, h=576, x=0, y=0) at libvo/vo_vdpau.c:986
> #5  0x08241096 in draw_slice (s=0xb70c870, src=0xad847d0, offset=0xbfc6efec, y=0, type=3, height=576) at libmpcodecs/vd_ffmpeg.c:519
> #6  0x0844edd3 in ff_draw_horiz_band (s=0xb71ff80, y=0, h=576) at mpegvideo.c:2117
> #7  0x0850520c in ff_vdpau_mpeg_picture_complete (s=0xb71ff80, buf=0xb6953008 "", buf_size=40001, slice_count=36) at vdpau.c:245
> #8  0x084267ee in decode_chunks (avctx=0xb70c870, picture=0xb70c7a0, data_size=0xbfc6f284, buf=0xb6953008 "", buf_size=40001) at mpeg12.c:2301
> #9  0x08426d26 in mpeg_decode_frame (avctx=0xb70c870, data=0xb70c7a0, data_size=0xbfc6f284, avpkt=0xbfc6f230) at mpeg12.c:2272
> #10 0x084ee9d5 in avcodec_decode_video2 (avctx=0xb70c870, picture=0xb70c7a0, got_picture_ptr=0xbfc6f284, avpkt=0xbfc6f230) at utils.c:611
> #11 0x08240344 in decode (sh=0xad82eb0, data=0xb6953008, len=40001, flags=0) at libmpcodecs/vd_ffmpeg.c:826
> #12 0x08146fcf in decode_video (sh_video=0xad82eb0, start=0xb6953008 "", in_size=40001, drop_frame=0, pts=0.23999999463558197) at libmpcodecs/dec_video.c:412
> #13 0x080c35fb in update_video (blit_frame=0xbfc72564) at mplayer.c:2398
> #14 0x080c788d in main (argc=4, argv=0xbfc72634) at mplayer.c:3823
Hm, could you test with some added sanity checks?
If that works, maybe remove the vl_vlc_fillbits call I added in vl_mpeg12_bs_decode
to see if that is what caused it. Unfortunately the bitstream parser just fails to work
correctly here on a lot of my test videos, but that happens even without this patch.
You might want to check with valgrind to see if it tosses any warning, too.
I don't suppose you have a short clip of the failing video that reproduces the problem?

diff --git a/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c b/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
index ddeaf31..2e95da6 100644
--- a/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
+++ b/src/gallium/auxiliary/vl/vl_mpeg12_bitstream.c
@@ -924,7 +924,7 @@ decode_slice(struct vl_mpg12_bs *bs)
          mb.coded_block_pattern = 0;
 
       vl_vlc_fillbits(&bs->vlc);
-   } while (vl_vlc_bits_left(&bs->vlc) && vl_vlc_peekbits(&bs->vlc, 23));
+   } while (vl_vlc_bits_left(&bs->vlc) >= 23 && vl_vlc_peekbits(&bs->vlc, 23));
 
    mb.num_skipped_macroblocks = 0;
    bs->decoder->decode_macroblock(bs->decoder, NULL, NULL, &mb.base, 1);
@@ -966,6 +966,7 @@ vl_mpg12_bs_decode(struct vl_mpg12_bs *bs,
    vl_vlc_init(&bs->vlc, n, len, buffer, lens);
 
    while (vl_vlc_bits_left(&bs->vlc) >= 32) {
+      vl_vlc_fillbits(&bs->vlc);
       if (vl_vlc_peekbits(&bs->vlc, 24) == 0x000001) {
          vl_vlc_eatbits(&bs->vlc, 24);
          if (vl_vlc_get_uimsbf(&bs->vlc, 8) > 0xaf)
diff --git a/src/gallium/auxiliary/vl/vl_vlc.h b/src/gallium/auxiliary/vl/vl_vlc.h
index e710862..ca18823 100644
--- a/src/gallium/auxiliary/vl/vl_vlc.h
+++ b/src/gallium/auxiliary/vl/vl_vlc.h
@@ -38,7 +38,7 @@
 struct vl_vlc
 {
    uint64_t buffer;
-   unsigned valid_bits;
+   int valid_bits;
    uint32_t *data;
    uint32_t *end;
 };
@@ -74,10 +74,12 @@ vl_vlc_init_table(struct vl_vlc_entry *dst, unsigned dst_size, const struct vl_v
 static INLINE void
 vl_vlc_fillbits(struct vl_vlc *vlc)
 {
+   assert(vlc->valid_bits >= 0);
+   assert(vlc->valid_bits <= 64);
    if (vlc->valid_bits < 32) {
       uint32_t value = *vlc->data;
 
-      //assert(vlc->data <= vlc->end);
+      assert(vlc->data <= vlc->end);
 
 #ifndef PIPE_ARCH_BIG_ENDIAN
       value = util_bswap32(value);
@@ -116,7 +118,7 @@ vl_vlc_init(struct vl_vlc *vlc,
    vl_vlc_fillbits(vlc);
 }
 
-static INLINE unsigned
+static INLINE int
 vl_vlc_bits_left(struct vl_vlc *vlc)
 {
    signed bytes_left = ((uint8_t*)vlc->end)-((uint8_t*)vlc->data);
@@ -126,7 +128,7 @@ vl_vlc_bits_left(struct vl_vlc *vlc)
 static INLINE unsigned
 vl_vlc_peekbits(struct vl_vlc *vlc, unsigned num_bits)
 {
-   //assert(vlc->valid_bits >= num_bits);
+   assert(vlc->valid_bits >= num_bits);
 
    return vlc->buffer >> (64 - num_bits);
 }
@@ -134,7 +136,7 @@ vl_vlc_peekbits(struct vl_vlc *vlc, unsigned num_bits)
 static INLINE void
 vl_vlc_eatbits(struct vl_vlc *vlc, unsigned num_bits)
 {
-   //assert(vlc->valid_bits > num_bits);
+   assert(vlc->valid_bits > num_bits);
 
    vlc->buffer <<= num_bits;
    vlc->valid_bits -= num_bits;
@@ -145,7 +147,7 @@ vl_vlc_get_uimsbf(struct vl_vlc *vlc, unsigned num_bits)
 {
    unsigned value;
 
-   //assert(vlc->valid_bits >= num_bits);
+   assert(vlc->valid_bits >= num_bits);
 
    value = vlc->buffer >> (64 - num_bits);
    vl_vlc_eatbits(vlc, num_bits);
@@ -158,7 +160,7 @@ vl_vlc_get_simsbf(struct vl_vlc *vlc, unsigned num_bits)
 {
    signed value;
 
-   //assert(vlc->valid_bits >= num_bits);
+   assert(vlc->valid_bits >= num_bits);
 
    value = ((int64_t)vlc->buffer) >> (64 - num_bits);
    vl_vlc_eatbits(vlc, num_bits);