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

List:       helix-datatype-dev
Subject:    Re: [datatype-dev] CR: GetPacket call re-entrancy failure in
From:       Eric Hyche <ehyche () real ! com>
Date:       2010-01-26 16:35:03
Message-ID: 04CB7997-C5A9-428E-84CA-C8725CE1B65D () real ! com
[Download RAW message or body]

Looks good.

On Jan 26, 2010, at 11:16 AM, <rajesh.rathinasamy@nokia.com> \
<rajesh.rathinasamy@nokia.com> wrote:

> 
> 
> 
> "Nokia submits this code under the terms of a commercial contribution agreement \
> with RealNetworks, and I am authorized to contribute this code under said \
> agreement." 
> Modified by:  rajesh.rathinasamy@nokia.com
> 
> Reviewed by:
> 
> Date: 25-jan-2010
> 
> Project:  SymbianMmf
> 
> Error Id:  DALM-7ZEU4L
> 
> Synopsis: GetPacket call re-entrancy failure in fileformats
> 
> Overview: 
> Recent change in file source for async I/O is to call GetPacket within PacketReady \
> callback. Some fileformats does not handle re-entrancy. 
> AVi fileformat ended up in calling ReadHeader on RIFF twice consequtively leading \
> to wrong section of data being interpreted as chunk header. This resulted in huge \
> file seek offset. Added a state in AVIState which is set and reset during \
> PacketReady callback and avoided the ScanState when in PacketReady sent state. 
> XPSFileformat had the issue of losing the getpacket outstanding flag value. Now the \
> flag is reset before passing the packet ready call to filesource. 
> Following Fileformats were tested with this changes:
> 3GP / Mp4, rm, asf, mp3, xps, mkv
> 
> Also tested thumbnail for avi, 3gp, wmv and rm.
> 
> 
> Files removed: None
> 
> Files added:  None
> 
> Files modified: 
> datatype/avi/fileformat/aviffpln.cpp
> datatype/avi/fileformat/pub/aviffpln.h
> datatype/xps/fileformat/CXPSFileformat.cpp
> 
> 
> Image size and heap use impact:  None
> 
> Module release testing (STIF):  PASSED. Tested on 5.0 as STIF is crashing on 9.2
> 
> Test case(s) added:  No
> 
> Memory leak check performed:  No. Unable to execute due to existing leaks.
> 
> Platforms and profiles build verified: helix-client-s60-50-mmf-mdf-dsp, \
> helix-client-s60-52-mmf-mdf-dsp 
> Platforms and profiles functionality verified:  armv5, winscw
> 
> Branch:  210CayS, HEAD, 420Brizo
> 
> Index: aviffpln.cpp
> ===================================================================
> RCS file: /cvsroot/datatype/avi/fileformat/aviffpln.cpp,v
> retrieving revision 1.5.6.4
> diff -w -u -b -r1.5.6.4 aviffpln.cpp
> --- aviffpln.cpp        5 Jan 2010 17:41:09 -0000       1.5.6.4
> +++ aviffpln.cpp        26 Jan 2010 01:05:33 -0000
> @@ -691,7 +691,11 @@
> m_pFFResponse->StreamDone(unStreamNumber);
> }
> 
> +    // Re-entrancy Check
> +    if(m_state != AS_SendingPacketReady)
> +    {
> ScanState();
> +    }
> 
> return HXR_OK;
> }
> @@ -1539,7 +1543,7 @@
> }
> else
> {
> -                               m_pFFResponse->PacketReady(pnr, NULL);
> +                               SendPacketReady(pnr, NULL);
> }
> }
> break;
> @@ -1742,7 +1746,7 @@
> 
> HXLOGL4(HXLOG_AVIX,"CAVIFileFormat[%p]::ScanState\tPacketReady, stream: \
> %lu\ttimestamp: %lu", this, pPending Packet->GetStreamNumber(), \
> pPendingPacket->GetTime()); // Warning: core call
> -                m_pFFResponse->PacketReady(HXR_OK, pPendingPacket);
> +                SendPacketReady(HXR_OK, pPendingPacket);
> HX_RELEASE(pPendingPacket);
> 
> // Reentrancy ch eck; if we're closed, return:
> @@ -2086,3 +2090,29 @@
> {
> return ((char) ulChunkId - '0')*10 + ((char) (ulChunkId >> 8) - '0');
> }
> +
> +void CAVIFileFormat::SendPacketReady(HX_RESULT rv, IHXPacket* pPacket)
> +{
> +
> +  if(m_pFFResponse != NULL)
> +  {
> +    // Store current state
> +    AVIState PrevState = m_state;
> +    m_state = AS_SendingPacketReady;
> +
> +    m_pFFResponse->PacketReady(rv, pPacket);
> +
> +    // Check & restore if the state is still the same and not changed because of
> +    // other re-entrant calls. Eg. AS_Closed.
> +    if(m_state == AS_SendingPacketReady)
> +    {
> +
> +      m_state = PrevState;
> +    }
> +    else
> +    {
> +      HXLOGL1(HXLOG_AVIX,"CAVIFileFormat[%p]::SendPacketReady() WARNING \
> PrevState:%d CurrState:%d", this, PrevState, m_state );
> +    }
> +  } // End of if(m_pFFResponse != NULL)
> +
> +}
> cvs diff: Diffing platform
> cvs diff: Diffing platform/mac
> cvs diff: Diffing platform/win
> cvs diff: Diffing pub
> Index: pub/aviffpln.h
> ===================================================================
> RCS file: /cvsroot/datatype/avi/fileformat/pub/aviffpln.h,v
> retrieving revision 1.4.6.2
> diff -w -u -b -r1.4.6.2 aviffpln.h
> --- pub/aviffpln.h      2 Dec 2009 18:37:13 -0000       1.4.6.2
> +++ pub/aviffpln.h      26 Jan 2010 01:05:33 -0000
> @@ -318,6 +318,8 @@
> 
> void IOEvent();
> 
> +    void SendPacketReady(HX_RESULT rv, IHXPacket* pPacket);
> +
> char*  m_pszTitle;
> char*  m_pszAuthor;
> char*  m_pszCopyright;
> @@ -393,6 +395,7 @@
> , AS_GetODMLStreamFilePending // used to initialise stream for ODML index case
> #endif
> , AS_IOEvent                // PacketReady
> +        , AS_SendingPacketReady     // Packet sent to fileformat response obj. \
> Added to handle re-entrancy , AS_Ready
> , AS_Closed
> }
> 
> 
> 
> Index: CXPSFileformat.cpp
> ===================================================================
> RCS file: /cvsroot/datatype/xps/fileformat/CXPSFileformat.cpp,v
> retrieving revision 1.1.1.1.2.16
> diff -w -u -b -r1.1.1.1.2.16 CXPSFileformat.cpp
> --- CXPSFileformat.cpp  14 Sep 2009 05:41:06 -0000      1.1.1.1.2.16
> +++ CXPSFileformat.cpp  26 Jan 2010 01:06:27 -0000
> @@ -907,8 +907,9 @@
> {
> HXLOGL4(HXLOG_SXPS, "CXPSFileFormat::DispatchPkt GetPkt[%lu] TS:%lu Seq:%lu",
> uStreamid, pPacket->GetTime(), m_parrStreams[uStreamid].GetLastPktSeqNo());
> -                m_pFFResponse->PacketReady(HXR_OK, pPacket);
> m_parrStreams[uStreamid].SetGetPktStatus(FALSE);
> +                m_pFFResponse->PacketReady(HXR_OK, pPacket);
> +
> 
> if( (m_parrStreams[uStreamid].IsStreamDone()) &&
> (m_parrStreams[uStreamid].IsStreamEmpty()) )
> 
> 
> <ATT00001..txt>

Eric Hyche (ehyche@real.com)
Principal Engineer
RealNetworks, Inc.



_______________________________________________
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]