[datatype-dev] CR:Fix bug 12720: A damaged AVI content caused all thumbnails can not be displayed
Li Qing lqing at real.comI checked into 310atlas, 361atlas and 362atlas. Do I need merge this patch to HEAD branch?
Best Regards!
Li Qing
--
RealNetworks China, Beijing Office
Tel: 008601059542848
Fax: 008601085656477
Address: 18th Floor,Tower B,Pacific Century Place,2A GongTiBeiLu,Chaoyang District,Beijing, China
Post Code: 100027
________________________________________
From: Qiang Luo
Sent: Friday, February 18, 2011 2:21 AM
To: Li Qing
Cc: datatype-dev at helixcommunity.org
Subject: Re: [datatype-dev] CR:Fix bug 12720: A damaged AVI content caused all thumbnails can not be displayed
1) The first fix is OK. Please define MAX_FRAMERATE to 120 and check
against it.
2) The second fix looks good to me. I was just asking you to explain
why "...it will waste some times and might bring negative impact".
Qiang
On 2/16/2011 7:24 PM, Li Qing wrote:
> For AVI video stream, the m_fChunksPerSecond means frame rate, and we use this parameter to calculate the target chunk and time stamp when seek/read in the stream, so if its value is wrong, then the program can't run correctly (btw, how about use 100 as the max frame rate?), the main codes are:
> void CAVIStream::Seek(UINT32 ulTime)
> {
> ...
> if (IsAudio ()&& m_header.ulSampleSize)
> {
> ....
> }
> else
> {
> ulTargetChunk = (UINT32) (ulTime / 1000.0 * m_fChunksPerSecond);
>
> if (FAILED(m_pIndex->FindClosestKeyChunk(m_usStream, ulTargetChunk, ulTargetKeyChunkOffset, ulTargetKeyChunk)))
> ...
> }
> ...
> if (IsAudio ()&& m_header.ulSampleSize)
> {
> ...
> }
> else
> {
> ulTime = (UINT32) ((m_ulMinReadChunk / m_fChunksPerSecond) * 1000);
> }
> ...
> }
> ...
> STDMETHODIMP CAVIStream::RIFFFindChunkDone(HX_RESULT status, UINT32 len)
> {
> ...
> if (IsAudio ()&& m_header.ulSampleSize)
> {
> ...
> }
> else
> {
> ulNextPacketTime = (UINT32) (((m_ulMaxReadChunk + 1)/ m_fChunksPerSecond) * 1000);
> }
> ...
> }
> STDMETHODIMP CAVIStream::RIFFReadDone(HX_RESULT status, IHXBuffer *pBuffer)
> {
> ...
> if (IsAudio ()&& m_header.ulSampleSize)
> {
> ...
> }
> else
> {
> ulTime = (UINT32) ((m_ulMaxReadChunk / m_fChunksPerSecond) * 1000);
> }
> ...
> }
>
> Since the program can't get the correct chunk, it invoke CAVIStream::GetNextSlice continually until it got to some unexpected status or the stream got to the EOF, and then it will waste some time.
> The logical error of the function CAVIStream::GetNextSlice is that the function CAVIIndex::FindClosestChunk will return HXR_NOT_INDEXABLE if there isn't any chunks in a slice, and in this case, we should finish seek/read in the stream, otherwise, the program will run into idle. I wouldn't modify the function CAVIIndex::FindClosestChunk because it is also invoked in function CAVIStream::GetMP3Packet.
>
> Best Regards!
> Li Qing
> --
> RealNetworks China, Beijing Office
> Tel: 008601059542848
> Fax: 008601085656477
> Address: 18th Floor,Tower B,Pacific Century Place,2A GongTiBeiLu,Chaoyang District,Beijing, China
> Post Code: 100027
> ________________________________________
> From: Qiang Luo
> Sent: Wednesday, February 16, 2011 10:47 PM
> To: Li Qing
> Cc: datatype-dev at helixcommunity.org
> Subject: Re: [datatype-dev] CR:Fix bug 12720: A damaged AVI content caused all thumbnails can not be displayed
>
> + if (m_fChunksPerSecond> 30)
> + {
> + result = HXR_UNEXPECTED;
> + goto On_Error;
> + }
>
> What is the m_fChunksPerSecond observed for this content? The number 30
> might be too low for future high frame rate contents.
>
> Could you elaborate on why the 2nd approach would impact performance?
>
> Qiang
>
> On 2/16/2011 4:06 AM, Li Qing wrote:
>> Modified by: lqing at real.com
>> Date: 02/16/2011
>> Project: RealPlayer for Android Smartphones
>>
>> Synopsis: Fix bug 12720: A damaged AVI content caused all thumbnails can not be displayed
>>
>> Overview: The content's time scale is wrong, it caused wrong frame rate, and the program will run into idle when capture the thumbnail of the damaged content.
>> I found 2 ways to fix this issue, one is returning an error code when the frame rate is wrong (avistrm.cpp.diff), and another is fixing the logical error in the function GetNextSlice (2_avistrm.cpp.diff).
>> The first patch is only for this content, so it shouldn't have any negative impact on the program, but can't fix other similar issues if some other properties are wrong. The second patch can avoid the program running into idle if some property is wrong, but it will waste some times and might bring negative impact. I can't decide which one is better, so I sent all of them out.
>>
>> Files Added: None
>>
>> Files Modified:
>> /datatype/avi/fileformat/avistrm.cpp
>>
>> Image Size and Heap Use impact (Client -Only): None
>>
>> Platforms and Profiles Affected:
>> Platform: android-2.2-arm-qsd_8250, android-2.2-arm-qsd_8650a
>> Profile: helix-client-android-full
>>
>> Distribution Libraries Affected:None
>>
>> Distribution library impact and planned action:None
>>
>> Platforms and Profiles Build Verified:
>> Platform: android-2.2-arm-qsd_8250
>> Profile: helix-client-android-full
>>
>> Platforms and Profiles Functionality verified:
>> Platform: android-2.2-arm-qsd_8250
>> Profile: helix-client-android-full
>>
>> Branch: HEAD, 310atlas, 361atlas, 362atlas
>>
>> Copyright assignment: I am a RealNetworks employee or contractor
>>
>> Best Regards!
>> Li Qing
>> --
>> RealNetworks China, Beijing Office
>> Tel: 008601059542848
>> Fax: 008601085656477
>> Address: 18th Floor,Tower B,Pacific Century Place,2A GongTiBeiLu,Chaoyang District,Beijing, China
>> Post Code: 100027
> .
>