[prev in list] [next in list] [prev in thread] [next in thread] 

List:       helix-datatype-dev
Subject:    [datatype-dev] CN:Bug 10053: force close after seeking a 3gp video
From:       "manju" <mmanjunath () real ! com>
Date:       2010-02-23 5:46:24
Message-ID: 001501cab449$ddf82580$1b01a8c0 () com2
[Download RAW message or body]


> Alright. The detection of wrap around is fine then.
-Checked into 361atlas for now.

>Will it still crash if the corrupted duration value is significantly large
>(eg. larger than clip duration) but not large enough to cause wrap-around? 
I tried few it doesn't crash.

Thanks,
-Manju

>fxd

-----Original Message-----
From: "manju" <mmanjunath@real.com>
To: "Xiaodong (Sheldon) Fu" <sfu@real.com>
Cc: "datatype-dev@helixcommunity.org" <datatype-dev@helixcommunity.org>
Sent: 2/22/10 1:26 AM
Subject: RE: [datatype-dev] CR:Bug 10053: force close after seeking a 3gp
video captured by camcorder


> Then we probably should have logic to guard against corrupted STTS 
> entries. SampleDuration should normally be very small, especially for 
> video since it is the inverse of frame rate. It should be safe to at 
> least limit it to be within the duration of the clip.

Well I looked into this; this is not possible with the current structure.
To limit the size to clip duration I need the header duration or 'mvhd'
atom, since I am a static object 'CQT_TimeToSample_Manager' called from
'CQTTrack' (has video track details for this instance) which essentially
passes only 'stbl' atom to me when init() function is called from
'CQTTrack'. 
'CQTTrack' does have video track header, but since clip duration is maximum
of audio/video duration entry, not reliable, and still
'CQT_TimeToSample_Manager' doesn't have this even!


> Also SamplesLeftInEntry is not one of the data field in the STTS table. 
> Where did you print out those two values? It's not clear whether the 
> data in the original file is corrupted or we have some issue with our 
> parsing code for STTS or somewhere else. What does STTS table in the 
> file itself look like? Is this corrupted entry the last one?

I just used 'SamplesLeftInEntry' name as it is from helix,
Actually it is, from qtff spec:
Time-to-sample table:
Sample count = 1
Sample duration = -4294909

I used mp4browser to see the above corrupted values in the file.
There are total 42699 entries in 'stts' and corrupt entry is present in
33383'Th index.
We don't have any problem in parsing; corrupt entry is in the original file
itself. 

Index: qtatmmgs.h
===================================================================
RCS file: /cvsroot/datatype/mp4/fileformat/pub/qtatmmgs.h,v
retrieving revision 1.19.8.9.8.1
diff -u -r1.19.8.9.8.1 qtatmmgs.h
--- qtatmmgs.h	7 Jan 2010 07:39:22 -0000	1.19.8.9.8.1
+++ qtatmmgs.h	22 Feb 2010 05:39:25 -0000
@@ -117,7 +117,14 @@
 
     HXBOOL AdvanceByMediaTime(ULONG32 ulMediaTime, HXBOOL &bDone)  // media
units
     {
-	m_ulCurrentInEditTime += ulMediaTime;
+	if(ulMediaTime + m_ulCurrentInEditTime < ulMediaTime) //check
overflow
+	{
+		m_ulCurrentInEditTime = 0xffffffff;
+	}
+	else
+	{
+		m_ulCurrentInEditTime += ulMediaTime;
+	}
 	bDone = FALSE; // Assume Not Done
 

>fxd

manju wrote:
> Thanks Sheldon for the review.
>
> I found the root cause for this.
> Actually one of the SampleDuration entries in stts atom is wrong or has
big
> signed entry.
>
> In video track, under stts atom, entry index 33383 has values:
> SamplesLeftInEntry = 1
> SampleDuration = -4294909
>
> So when we parse this value as unsigned we get SampleDuration as
4290672387
> and in turn lead to the bigger values we are seeing for others.
>
> Do you like the earlier fix (overflow check and assign maximum it can
hold)
> or any suggestions for this case?
>
> Thanks,
> -Manju
>
> -----Original Message-----
> From: Sheldon Fu [mailto:sfu@real.com] 
> Sent: Thursday, February 18, 2010 9:11 PM
> To: manju
> Cc: datatype-dev@helixcommunity.org
> Subject: Re: [datatype-dev] CR:Bug 10053: force close after seeking a 3gp
> video captured by camcorder
>
> You can detect overflow faster by simply do:
>
> if(ulMediaTime + m_ulCurrentInEditTime < ulMediaTime) //check overflow
>
> And here doesn't seem to be the root cause of the problem. 32-bit 
> integer holding ms value will wrap around in more than 1 month, why 
> could m_ulCurrentInEditTime get to that big a value in the first place? 
> Maybe the fix should be there to prevent m_ulCurrentInEditTime to get to 
> that big.
>
> fxd
>
> manju wrote:
>   
>> Synopsis:
>> Force close after seeking a 3gp video captured by camcorder
>>
>> Overview:
>> File Type: 3gp
>> File duration in header: 93.53 min
>> Audio duration header: 93.53 min
>> Video duration in header: 28.33 min
>>
>> When we try to seek/play this particular file after 70+ min (around 74
>>     
> min),
>   
>> results in low memory and phone resets!
>> The issue was overflow of a variable in mp4 fileformat.
>> Say suppose we seek to 4800000ms (80min), at this point, for this
content,
>> we are not able to locate the prevkeyframe and hence we advances to
>> AdvanceByMediaTime() where we add current edit time and last media sync
>>     
> time
>   
>> which will be very big at that time (perhaps could not sync the value).
>> So, now when we add current edit time and last media sync time
>> (m_ulCurrentInEditTime += ulMediaTime;), memory will be overflowed and
>> results in garbage value and eventually generation of only large chunks
of
>> video packets and drying up the memory.
>> So overflow check has been made to bail out of these cases.
>>
>> Files Modified:
>> datatype/mp4/fileformat/pub/qtatmmgs.h
>>
>> Files Attached:
>> diff.txt
>>
>> Branch: 361atlas
>> -Verified on Android board for 361atlas branch.
>>
>> Thanks,
>> -Manju
>>   
>>     
>
>
> .
>
>   





_______________________________________________
Datatype-dev mailing list
Datatype-dev@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/datatype-dev
[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic