[FFmpeg-devel] ffplay segfaults on invalid h264 stream

[Panagiotis Issaris]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

Michael Niedermayer wrote:
> On Thu, May 03, 2007 at 05:53:47PM +0200, Panagiotis Issaris wrote:
>> ffplay segfaults on a specific stream I'm trying to decode. I'm
>> using revision 8880.
>>
> [...]
>>
>> A simple fix for this is attached. I am far from sure that this is the
>> correct way to fix it, but it might help illustrating the problem.
>>
>> I can also provide the sample which causes the crash, although the
>> previous times I haven't been successful in getting these samples were
>> they belong.
> [...]
>> diff --git a/libavcodec/h264.c b/libavcodec/h264.c
>> index f8f61d3..c7a96b4 100644
>> --- a/libavcodec/h264.c
>> +++ b/libavcodec/h264.c
>> @@ -4397,6 +4397,7 @@ static int decode_ref_pic_marking(H264Context *h){
>>          }else{
>>              assert(h->long_ref_count + h->short_ref_count <= h->sps.ref_frame_count);
>>  
>> +            if((h->short_ref_count>0) && h->short_ref[h->short_ref_count-1] && (h->long_ref_count + h->short_ref_count == h->sps.ref_frame_count)){ //FIXME fields
>>              if(h->long_ref_count + h->short_ref_count == h->sps.ref_frame_count){ //FIXME fields
>>                  h->mmco[0].opcode= MMCO_SHORT2UNUSED;
>>                  h->mmco[0].short_frame_num= h->short_ref[ h->short_ref_count - 1 ]->frame_num;
> 
> missmatching {

Oops, the added line was supposed to replace the line following it...



> h.264 spec says:
> ----
> adaptive_ref_pic_marking_mode_flag selects the reference picture marking mode of the currently decoded picture as
> specified in Table 7-8. adaptive_ref_pic_marking_mode_flag shall be equal to 1 when the number of frames,
> complementary field pairs, and non-paired fields that are currently marked as "used for long-term reference" is equal to
> Max( num_ref_frames, 1 ).
> ----
> 
> so the h->short_ref_count>0 seems wrong its an error condition if its false
> not a no-op condition

I see. Should the decoder just bail out if the stream is invalid, or
should it try to decode it anyway (thus adding some error tolerance)? I
am not sure how difficult it will be to have true error tolerance: With
the patch above my broken bitstream keeps decoding although the image is
"a bit" corrupted. But, the decoder segfaults somewhere later in the
stream where another null pointer is dereferenced. In that case the
pointer that is dereferenced is not always zero when it is corrupted;
it's sometimes 10 or some other obviously wrong value.



> the h->short_ref[h->short_ref_count-1] check also seems wrong as 
> h->short_ref_count of X implicates that there are X entries in h->short_ref

Isn't it possible that a corrupted stream has an invalid
h->short_ref_count? And if so, how should I handle it?


With friendly regards,
Takis
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGOvA49kOxLuzz4CkRAu1vAJ9p0FHXRwf3PWGPJfdqqbsQWlYCBQCfQlcc
qSdqOAZza5VnPCYaCb6vN3g=
=90iM
-----END PGP SIGNATURE-----