[FFmpeg-devel] [PATCH] dv: fix valgrind use of uninitialised value warnings.

[Reimar Döffinger]

On Mon, Jul 04, 2011 at 11:40:34AM +0200, Clément Bœsch wrote:
> From: Clément Bœsch <clement.boesch at smartjog.com>
> 
> ---
> Hi,
> 
> As the subject says, the patch fixes valgrind warnings I expect with a DV100
> file. I'm not sure this is a false positive from valgrind thought, and
> unfortunately the sample I use can't be shared.
> 
> Here is an extract of expected warnings:
> 
>     ==8399== Use of uninitialised value of size 4
>     ==8399==    at 0x82800A4: dv_decode_ac (dv.c:413)
>     ==8399==    by 0x82840EB: dv_decode_video_segment (dv.c:550)
>     ==8399==    by 0x8482A85: avcodec_default_execute (utils.c:440)
>     ==8399==    by 0x8280BB2: dvvideo_decode_frame (dv.c:1098)
>     ==8399==    by 0x8483611: avcodec_decode_video2 (utils.c:747)
>     ==8399==    by 0x804CB59: output_packet (ffmpeg.c:1611)
>     ==8399==    by 0x808D2A1: transcode.constprop.24 (ffmpeg.c:2781)
>     ==8399==    by 0x8087A75: main (ffmpeg.c:4583)
>     ==8399== 
>     ==8399== Use of uninitialised value of size 4
>     ==8399==    at 0x828011F: dv_decode_ac (dv.c:410)
>     ==8399==    by 0x82840EB: dv_decode_video_segment (dv.c:550)
>     ==8399==    by 0x8482A85: avcodec_default_execute (utils.c:440)
>     ==8399==    by 0x8280BB2: dvvideo_decode_frame (dv.c:1098)
>     ==8399==    by 0x8483611: avcodec_decode_video2 (utils.c:747)
>     ==8399==    by 0x804CB59: output_packet (ffmpeg.c:1611)
>     ==8399==    by 0x808D2A1: transcode.constprop.24 (ffmpeg.c:2781)
>     ==8399==    by 0x8087A75: main (ffmpeg.c:4583)
>     ==8399== 
>     ==8399== Use of uninitialised value of size 4e=00:00:01.06 bitrate=2804.1kbits/s    
>     ==8399==    at 0x8280111: dv_decode_ac (dv.c:408)
>     ==8399==    by 0x8284356: dv_decode_video_segment (dv.c:572)
>     ==8399==    by 0x8482A85: avcodec_default_execute (utils.c:440)
>     ==8399==    by 0x8280BB2: dvvideo_decode_frame (dv.c:1098)
>     ==8399==    by 0x8483611: avcodec_decode_video2 (utils.c:747)
>     ==8399==    by 0x804CB59: output_packet (ffmpeg.c:1611)
>     ==8399==    by 0x808D2A1: transcode.constprop.24 (ffmpeg.c:2781)
>     ==8399==    by 0x8087A75: main (ffmpeg.c:4583)
>     ==8399== 
> 
> Note: it is not a regression (the issue is present since the first DV100
> support was added).
> 
> ---
>  libavcodec/dv.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavcodec/dv.c b/libavcodec/dv.c
> index d6c49c8..f426d73 100644
> --- a/libavcodec/dv.c
> +++ b/libavcodec/dv.c
> @@ -476,8 +476,8 @@ static int dv_decode_video_segment(AVCodecContext *avctx, void *arg)
>      GetBitContext gb;
>      BlockInfo mb_data[5 * DV_MAX_BPM], *mb, *mb1;
>      LOCAL_ALIGNED_16(DCTELEM, sblock, [5*DV_MAX_BPM], [64]);
> -    LOCAL_ALIGNED_16(uint8_t, mb_bit_buffer, [80 + 4]); /* allow some slack */
> -    LOCAL_ALIGNED_16(uint8_t, vs_bit_buffer, [5 * 80 + 4]); /* allow some slack */
> +    LOCAL_ALIGNED_16(uint8_t, mb_bit_buffer, [80 + 4])     = {0}; /* allow some slack */
> +    LOCAL_ALIGNED_16(uint8_t, vs_bit_buffer, [5 * 80 + 4]) = {0}; /* allow some slack */

This seems wasteful, how about something like below instead (untested)?
diff --git a/libavcodec/dv.c b/libavcodec/dv.c
index d6c49c8..dfa66e7 100644
--- a/libavcodec/dv.c
+++ b/libavcodec/dv.c
@@ -476,8 +476,8 @@ static int dv_decode_video_segment(AVCodecContext *avctx, void *arg)
     GetBitContext gb;
     BlockInfo mb_data[5 * DV_MAX_BPM], *mb, *mb1;
     LOCAL_ALIGNED_16(DCTELEM, sblock, [5*DV_MAX_BPM], [64]);
-    LOCAL_ALIGNED_16(uint8_t, mb_bit_buffer, [80 + 4]); /* allow some slack */
-    LOCAL_ALIGNED_16(uint8_t, vs_bit_buffer, [5 * 80 + 4]); /* allow some slack */
+    LOCAL_ALIGNED_16(uint8_t, mb_bit_buffer, [80 + 8]); /* allow some slack */
+    LOCAL_ALIGNED_16(uint8_t, vs_bit_buffer, [5 * 80 + 8]); /* allow some slack */
     const int log2_blocksize = 3-s->avctx->lowres;
     int is_field_mode[5];
 
@@ -544,6 +544,7 @@ static int dv_decode_video_segment(AVCodecContext *avctx, void *arg)
         block = block1;
         mb    = mb1;
         init_get_bits(&gb, mb_bit_buffer, put_bits_count(&pb));
+        put_bits32(&pb, 0);
         flush_put_bits(&pb);
         for (j = 0; j < s->sys->bpm; j++, block += 64, mb++) {
             if (mb->pos < 64 && get_bits_left(&gb) > 0) {
@@ -564,6 +565,7 @@ static int dv_decode_video_segment(AVCodecContext *avctx, void *arg)
     block = &sblock[0][0];
     mb    = mb_data;
     init_get_bits(&gb, vs_bit_buffer, put_bits_count(&vs_pb));
+    put_bits32(&vs_pb, 0);
     flush_put_bits(&vs_pb);
     for (mb_index = 0; mb_index < 5; mb_index++) {
         for (j = 0; j < s->sys->bpm; j++) {