On Mon, Jul 04, 2011 at 09:46:12AM -0700, Ronald S. Bultje wrote:
[...]
> >
> > ---
> >  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 is a little redundant, but it's true that each input buffer needs
> zero padding and this one doesn't. The easiest solution, I think, is
> to add zero padding (plus add the put_flush_bits()?) at the end after
> the written bytes (FF_INPUT_BUFFER_PADDING_SIZE or so), s/4/that/ also
> above) in bit_copy().
> 

Thanks, see attached version, slightly modified from patch proposed by
Reimar. I confirmed it works here. Also, it's pointless to do it in
bit_copy, just padding at that moment (when {mb,vs}_bit_buffer are
completely filled and will start being read) is enough.

I also attach a second patch fixing various comments I expected while
spending time browsing that file.

-- 
Clément B.
From 81b5d79dab2e11e6fbb33192468527919ad5d783 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <[email protected]>
Date: Tue, 5 Jul 2011 10:30:48 +0200
Subject: [PATCH 1/2] dv: fix various comments.

---
 libavcodec/dv.c |   19 +++++++++----------
 1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/libavcodec/dv.c b/libavcodec/dv.c
index d6c49c8..b6f7852 100644
--- a/libavcodec/dv.c
+++ b/libavcodec/dv.c
@@ -370,7 +370,6 @@ typedef struct BlockInfo {
 
 /* bit budget for AC only in 5 MBs */
 static const int vs_total_ac_bits = (100 * 4 + 68*2) * 5;
-/* see dv_88_areas and dv_248_areas for details */
 static const int mb_area_start[5] = { 1, 6, 21, 43, 64 };
 
 static inline int put_bits_left(PutBitContext* s)
@@ -378,7 +377,7 @@ static inline int put_bits_left(PutBitContext* s)
     return (s->buf_end - s->buf) * 8 - put_bits_count(s);
 }
 
-/* decode ac coefficients */
+/* decode AC coefficients */
 static void dv_decode_ac(GetBitContext *gb, BlockInfo *mb, DCTELEM *block)
 {
     int last_index = gb->size_in_bits;
@@ -391,7 +390,7 @@ static void dv_decode_ac(GetBitContext *gb, BlockInfo *mb, 
DCTELEM *block)
     OPEN_READER(re, gb);
     UPDATE_CACHE(re, gb);
 
-    /* if we must parse a partial vlc, we do it here */
+    /* if we must parse a partial VLC, we do it here */
     if (partial_bit_count > 0) {
         re_cache = ((unsigned)re_cache >> partial_bit_count) |
                    (mb->partial_bit_buffer << (sizeof(re_cache) * 8 - 
partial_bit_count));
@@ -486,7 +485,7 @@ static int dv_decode_video_segment(AVCodecContext *avctx, 
void *arg)
 
     memset(sblock, 0, 5*DV_MAX_BPM*sizeof(*sblock));
 
-    /* pass 1 : read DC and AC coefficients in blocks */
+    /* pass 1: read DC and AC coefficients in blocks */
     buf_ptr = &s->buf[work_chunk->buf_offset*80];
     block1  = &sblock[0][0];
     mb1     = mb_data;
@@ -503,7 +502,7 @@ static int dv_decode_video_segment(AVCodecContext *avctx, 
void *arg)
             last_index = s->sys->block_sizes[j];
             init_get_bits(&gb, buf_ptr, last_index);
 
-            /* get the dc */
+            /* get the DC */
             dc       = get_sbits(&gb, 9);
             dct_mode = get_bits1(&gb);
             class1   = get_bits(&gb, 2);
@@ -530,7 +529,7 @@ static int dv_decode_video_segment(AVCodecContext *avctx, 
void *arg)
             av_dlog(avctx, "MB block: %d, %d ", mb_index, j);
             dv_decode_ac(&gb, mb, block);
 
-            /* write the remaining bits  in a new buffer only if the
+            /* write the remaining bits in a new buffer only if the
                block is finished */
             if (mb->pos >= 64)
                 bit_copy(&pb, &gb);
@@ -539,7 +538,7 @@ static int dv_decode_video_segment(AVCodecContext *avctx, 
void *arg)
             mb++;
         }
 
-        /* pass 2 : we can do it just after */
+        /* pass 2: we can do it just after */
         av_dlog(avctx, "***pass 2 size=%d MB#=%d\n", put_bits_count(&pb), 
mb_index);
         block = block1;
         mb    = mb1;
@@ -559,7 +558,7 @@ static int dv_decode_video_segment(AVCodecContext *avctx, 
void *arg)
             bit_copy(&vs_pb, &gb);
     }
 
-    /* we need a pass other the whole video segment */
+    /* we need a pass over the whole video segment */
     av_dlog(avctx, "***pass 3 size=%d\n", put_bits_count(&vs_pb));
     block = &sblock[0][0];
     mb    = mb_data;
@@ -640,7 +639,7 @@ static int dv_decode_video_segment(AVCodecContext *avctx, 
void *arg)
 }
 
 #if CONFIG_SMALL
-/* Converts run and level (where level != 0) pair into vlc, returning bit size 
*/
+/* Converts run and level (where level != 0) pair into VLC, returning bit size 
*/
 static av_always_inline int dv_rl2vlc(int run, int level, int sign, uint32_t* 
vlc)
 {
     int size;
@@ -817,7 +816,7 @@ static av_always_inline int dv_init_enc_block(EncBlockInfo* 
bi, uint8_t *data, i
 
           if (level + 15 > 30U) {
               bi->sign[i] = (level >> 31) & 1;
-              /* weigh it and and shift down into range, adding for rounding */
+              /* weight it and and shift down into range, adding for rounding 
*/
               /* the extra division by a factor of 2^4 reverses the 8x 
expansion of the DCT
                  AND the 2x doubling of the weights */
               level = (FFABS(level) * weight[i] + (1 << (dv_weight_bits+3))) 
>> (dv_weight_bits+4);
-- 
1.7.5.4

From 33c44851bf544b2d60cc8dbd107375db8b128601 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Reimar=20D=C3=B6ffinger?= <[email protected]>
Date: Tue, 5 Jul 2011 16:02:56 +0200
Subject: [PATCH 2/2] dv: fix valgrind use of uninitialised value warnings.

---
 libavcodec/dv.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavcodec/dv.c b/libavcodec/dv.c
index b6f7852..2dc0373 100644
--- a/libavcodec/dv.c
+++ b/libavcodec/dv.c
@@ -475,8 +475,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 + 
FF_INPUT_BUFFER_PADDING_SIZE]); /* allow some slack */
+    LOCAL_ALIGNED_16(uint8_t, vs_bit_buffer, [5*80 + 
FF_INPUT_BUFFER_PADDING_SIZE]); /* allow some slack */
     const int log2_blocksize = 3-s->avctx->lowres;
     int is_field_mode[5];
 
@@ -543,6 +543,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); // padding must be zero'ed
         flush_put_bits(&pb);
         for (j = 0; j < s->sys->bpm; j++, block += 64, mb++) {
             if (mb->pos < 64 && get_bits_left(&gb) > 0) {
@@ -563,6 +564,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); // padding must be zero'ed
     flush_put_bits(&vs_pb);
     for (mb_index = 0; mb_index < 5; mb_index++) {
         for (j = 0; j < s->sys->bpm; j++) {
-- 
1.7.5.4

Attachment: pgpiXOTK9CP3z.pgp
Description: PGP signature

_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to