[FFmpeg-devel] [PATCH] Fix a valgrind error in h264.c

[Vitor Sessak]

Michael Niedermayer wrote:
> On Tue, Mar 03, 2009 at 11:24:14PM +0100, Vitor Sessak wrote:
>> Michael Niedermayer wrote:
>>> On Tue, Mar 03, 2009 at 10:41:33PM +0100, Michael Niedermayer wrote:
>>>> On Tue, Mar 03, 2009 at 08:57:15PM +0100, Vitor Sessak wrote:
>>>>> Hi,
>>>>>
>>>>> I was wondering why MinGW fails decoding CVMAPAQP3_Sony_E.jsv in FATE 
>>>>> (http://fate.multimedia.cx/index.php?build_record=61129), so I tested it 
>>>>> with valgrind in my box:
>>>>>
>>>>>> ==10039== Conditional jump or move depends on uninitialised value(s)
>>>>>> ==10039==    at 0x8146DD5: ff_print_debug_info (mpegvideo.c:1092)
>>>>>> ==10039== ==10039== Conditional jump or move depends on uninitialised 
>>>>>> value(s)
>>>>>> ==10039==    at 0x82A3971: h264_parse (h264_parser.c:55)
>>>>>> ==10039== ==10039== Invalid read of size 1
>>>>>> ==10039==    at 0x828BFBE: filter_mb (h264.c:6471)
>>>>>> ==10039==  Address 0x49374a3 is not stack'd, malloc'd or (recently) 
>>>>>> free'd
>>>>> The attached patch fixes the last one, but I have no familiarity with 
>>>>> h264 code (nor $bignum hours of free time to acquire it) so I don't know 
>>>>> if it is the right way to fix it...
>>>>>
>>>>> -Vitor
>>>>>
>>>>> PS: I don't know either if this actually fixes MinGW or not...
>>>>> Index: libavcodec/h264.c
>>>>> ===================================================================
>>>>> --- libavcodec/h264.c	(revision 16968)
>>>>> +++ libavcodec/h264.c	(working copy)
>>>>> @@ -6470,7 +6470,7 @@
>>>>>          int qp = s->current_picture.qscale_table[mb_xy];
>>>>>          if(qp <= qp_thresh
>>>>>             && (mb_x == 0 || ((qp + 
>>>>> s->current_picture.qscale_table[mb_xy-1] + 1)>>1) <= qp_thresh)
>>>>> -           && (mb_y == 0 || ((qp + 
>>>>> s->current_picture.qscale_table[h->top_mb_xy] + 1)>>1) <= qp_thresh)){
>>>>> +           && (mb_y == 0 || h->top_mb_xy < 0 || ((qp + 
>>>>> s->current_picture.qscale_table[h->top_mb_xy] + 1)>>1) <= qp_thresh)){
>>>> my gut feeling says the check should be
>>>> is h->top_mb_xy < 0 alone not enough?
>>> minor bug in my sentance generation code, i meant just
>>> is h->top_mb_xy < 0 alone not enough?
>> At least for this file, yes it is. New patch attached.
> 
> ok

Thanks for the review, applied.

-Vitor