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

List:       helix-filesystem-dev
Subject:    Re: [Client-dev] RE: [Filesystem-dev] Longer start-up time and buffer
From:       Sheldon Fu <sfu () real ! com>
Date:       2010-09-21 23:23:12
Message-ID: 4C993E60.2030801 () real ! com
[Download RAW message or body]

While it is true that the local filesystem API behaves in sync manner 
now, the same file source logic here is used for http streaming too, 
which will have async behavior so a solution in file source has to 
always take care of both cases.

The biggest long term problem of this change I think is the constant 
loop value. Even for Symbian, the optimal value  I think would be 
affected not just by Symbian OS but on what hardware it runs on, e.g. 
harddrive and flash memory will have different read speed.

fxd


Junhong.Liu@nokia.com wrote:
> Hi, Sheldon:
> 
> Thanks! I will propose my change for 420 brizo only.
> 
> To support 64 bit file system, we switched to async read.  I think all the current \
> reading logics in Brizo/HEAD file source assume sync read,  never think about async \
> read situation although it can support async read.   Actually my 5 packets event \
> queue behaves similar to what you described in file format pre-reads block, but \
> just in file source level. 
> Regards,
> 
> Junhong
> 
> 
> 
> -----Original Message-----
> From: ext Sheldon Fu [mailto:sfu@real.com]
> Sent: Tuesday, September 21, 2010 4:09 PM
> To: Liu Junhong (Nokia-MS/Dallas)
> Cc: Tony Seaward; filesystem-dev@helixcommunity.org; \
>                 datatype-dev@helixcommunity.org; \
>                 client-dev@lists.helixcommunity.org
> Subject: Re: [Client-dev] RE: [Filesystem-dev] Longer start-up time and buffer \
> starvation on 420 Brizo & Head 
> Looks good as a Symbian only fix/workaround.
> 
> For the Head version though, we would need a solution that works for both sync and \
> async PacketReady cases and something that doesn't depend on that empirical \
> constant loop value. 
> Curiously, why is it that on Symbian the PacketReady is always async?
> Even if the filesystem API (and the Helix filesystem plugin) is async, the file \
> format plugin GetPacket/PacketReady api can still behave in sync fashion. E.g, if \
> for performance reason the file format pre-reads a large block of data from the \
> file then it will be able to satisfy GetPacket calls for a while without turning \
> around to ask file system to read more data. This would result in at least some \
> sync PacketReady callbacks. 
> fx
> 
> Junhong.Liu@nokia.com wrote:
> 
> > Hi, Sheldon:
> > 
> > I tried many approaches, the change below works very well for us.  The basic idea \
> > is to start another FillBuffers()->GetPacket() in PacketReady() callback until \
> > max 5 packets are received.  Then an immediate sourceInfo/hxplayer callback is \
> > scheduled. In this immediate callback, these 5 packets will be moved to Renderer \
> > and another packet read will be issued if needed. So the overall CPU usage \
> > increase is very limited.  I tested this solution, it fixed long start-up/long \
> > seeking time, buffer starvation/re-buffering and Power save problems.  Responsive \
> > to users input is quick, which means CPU's idle is in good percentage.  I used \
> > Symbian flag to define the max packet event  (5) to match Symbian ^ 3 device's \
> > file I/O  performance. For other platforms, it's defined as 0, so no impact on \
> > them.  And the immediate callback in under the async read mode flag, so it won't \
> > impact the sync read logic. 
> > Could you comment on my fix?
> > 
> > Thanks,
> > 
> > Junhong
> > 
> > ===================================================================
> > RCS file: /cvsroot/client/core/hxflsrc.cpp,v
> > retrieving revision 1.138.12.1.2.2
> > diff -w -u -r1.138.12.1.2.2 hxflsrc.cpp
> > --- hxflsrc.cpp 5 Mar 2010 13:18:41 -0000       1.138.12.1.2.2
> > +++ hxflsrc.cpp 20 Sep 2010 15:58:56 -0000
> > @@ -109,6 +109,13 @@
> > #define SOURCE_TIMELINE_TOL  500
> > #define LOCALSOURCE_FAST_START_BW_CAPACITY_THRESHOLD   150     // A percentage of \
> > source bitrate 
> > +#ifdef _SYMBIAN
> > +#define MAX_NUM_EVENTS 5
> > +#else
> > +#define MAX_NUM_EVENTS 0
> > +#endif
> > +
> > +
> > HXFileSource::HXFileSource()
> > > m_lRefCount(0)
> > , m_ulLastBufferingReturned(0)
> > @@ -2804,8 +2811,16 @@
> > 
> > if(!m_bInFillMode)
> > {
> > +    if (lEventList->GetNumEvents() <= MAX_NUM_EVENTS)
> > +    {
> > FillBuffers(HX_GET_BETTERTICKCOUNT());
> > }
> > +    else
> > +    {
> > +        m_pSourceInfo->ScheduleProcessCallback();
> > +    }
> > +
> > +    }
> > 
> > if (!theErr)
> > {
> > @@ -3423,7 +3438,7 @@
> > 
> > if (!lpStreamInfo->m_bSrcStreamDone)
> > {
> > -           if (lpStreamInfo->m_EventList.GetNumEvents() == 0)
> > +           if (lpStreamInfo->m_EventList.GetNumEvents() <=
> > + MAX_NUM_EVENTS)
> > {
> > bHasUnfilledStreams = TRUE;
> > 
> > @@ -3439,7 +3454,7 @@
> > 
> > if (!lpStreamInfo->m_bPacketRequested)
> > {
> > -               if (lpStreamInfo->m_EventList.GetNumEvents() == 0)
> > +               if (lpStreamInfo->m_EventList.GetNumEvents() <=
> > + MAX_NUM_EVENTS)
> > {
> > lpStreamInfoOut = lpStreamInfo;
> > bSelectedStream = TRUE;
> > 
> > -----Original Message-----
> > From: ext Sheldon Fu [mailto:sfu@real.com]
> > Sent: Friday, September 10, 2010 9:20 AM
> > To: Liu Junhong (Nokia-MS/Dallas)
> > Cc: Tony Seaward; filesystem-dev@helixcommunity.org;
> > datatype-dev@helixcommunity.org; client-dev@lists.helixcommunity.org
> > Subject: RE: [Client-dev] RE: [Filesystem-dev] Longer start-up time
> > and buffer starvation on 420 Brizo & Head
> > 
> > Adding ProcessIdle call from PacketReady() will also need to be tested on all the \
> > platforms. Although code change is mimimum, its impact on the logical flow \
> > probably is even bigger than adding a loop at top level. 
> > If you don't think we can pull it off with a higher level loop and we
> > need to preserve the ability for ProcessIdle to regulate how much time
> > slice it can spend on each run, I'd like to propose we do this --
> > 
> > HXPlayer::ProcessIdle reschedules the callback at the end, normally for the \
> > predefined INTERVAL (30ms on most platforms). There is already a flag to \
> > SchedulePlayer call that can make the callback IMMEDIATE. What we could do is to \
> > add a member flag to HXPlay, e.g. m_bWantImmediateScheduling that a lower level \
> > function (e.g. PacketReady()) can set if it feels the need. 
> > m_bWantImmediateScheduling will just be a hint to ProcessIdle though. ProcessIdle \
> > can have some logic to measure how much time it has already spend on the current \
> > run and if it is about to use up all the allowance, it can reject the \
> > m_bWantImmediateScheduling hint and still schedule a regular callback. 
> > On Symbian if you don't really care for ProcessIdle to always yield some CPU \
> > time, you can always honor the m_bWantImmediateScheduling request, then it \
> > basically runs the same as your proposal. This will leave us the opportunity \
> > though to restore the time slice allowance logic on platforms that do need it. 
> > This is still quite inefficent I think since although we run at full speed now \
> > but we start from the top of ProcessIdle for EACH packet. ProcessIdle does a lot \
> > of other checks other than moving the packets from source to renderer. This is \
> > likely to get worse when we have to deal with high frame-rate content. 
> > fxd
> > 
> > ________________________________________
> > From: Junhong.Liu@nokia.com [Junhong.Liu@nokia.com]
> > Sent: Wednesday, September 08, 2010 5:47 PM
> > To: Sheldon Fu
> > Cc: Tony Seaward; filesystem-dev@helixcommunity.org; \
> >                 datatype-dev@helixcommunity.org; \
> >                 client-dev@lists.helixcommunity.org
> > Subject: RE: [Client-dev] RE: [Filesystem-dev] Longer start-up time and buffer \
> > starvation on 420        Brizo & Head 
> > Hi, Sheldon:
> > 
> > See my reply inline.
> > Thanks,
> > Junhong
> > 
> > 
> > -----Original Message-----
> > From: ext Sheldon Fu [mailto:sfu@real.com]
> > Sent: Wednesday, September 08, 2010 3:15 PM
> > To: Liu Junhong (Nokia-MS/Dallas)
> > Cc: Tony Seaward; filesystem-dev@helixcommunity.org;
> > datatype-dev@helixcommunity.org; client-dev@lists.helixcommunity.org
> > Subject: Re: [Client-dev] RE: [Filesystem-dev] Longer start-up time
> > and buffer starvation on 420 Brizo & Head
> > 
> > Oh, ic. Then the original design is to have each FillBuffers call only fetch one \
> > packet for each stream. The calling of FillBuffers from PacketReady is \
> > unnecessary and shouldn't be merged in. 
> > It seems to me that we somehow need a loop in the SourceInfo::ProcessIdle to keep \
> > fetching packets from source and feed to renderer up to either the predefined \
> > time-slice or preroll amount, otherwise I don't see how we can do preroll as fast \
> > as possible. [Junhong] I am afraid this change is too big, especially it needs to \
> > be tested on all platforms. 
> > Calling ProcessIdle from PacketReady will create a recursive chain of ProcessIdle \
> > I think when GetPacket runs in sync mode. If we choose to go with that approach, \
> > you'll have actually schedule a immediate ProcessIdle callback from PacketReady, \
> > instead of calling it directly. [Junhong]  We have m_bInFillMode flag to prevent \
> > the recursive call on ProcessIdle for sync read.  If you think we need to do it \
> > for sync read too, I can use an immediate ProcessIdle callback regardless sync or \
> > async read. 
> > 
> > 
> > > if(!m_bInFillMode)
> > > 
> > > 
> > > > {
> > > > -        FillBuffers(HX_GET_BETTERTICKCOUNT());
> > > > +       m_pPlayer->ProcessIdle(FALSE);
> > > > }
> > > > 
> > > > 
> > fxd
> > 
> > Junhong.Liu@nokia.com wrote:
> > 
> > 
> > > Yes, 'HC CR' is the original CR (Aug, 2006).  The original CR didn't call \
> > > FillBuffers from PacketReady.  Asheesh's CR (Feb, 2010) added it by merging \
> > > code from 210 CayS, see attached.  However the change of FillBuffers logic \
> > > prevents another GetPacket() call in FillBuffer() if the queue is not empty.   \
> > > You can check SelectNextStreamToFill(). If the queue is not empty, it will \
> > > always return FALSE.  So basically call FillBuffers() inside PacketReady() is \
> > > useless. We also tried to change SelectNextStreamToFill() to force it return \
> > > TRUE, so FillBuffers can issue GetPacket() even the queue is not empty. However \
> > > since all the buffer manager logics have been moved to HXPlayer level,  we \
> > > don't know when we should stop the read. 
> > > Regards,
> > > 
> > > Junhong
> > > 
> > > 
> > > -----Original Message-----
> > > From: ext Sheldon Fu [mailto:sfu@real.com]
> > > Sent: Wednesday, September 08, 2010 1:54 PM
> > > To: Liu Junhong (Nokia-MS/Dallas); Sheldon Fu
> > > Cc: Tony Seaward; filesystem-dev@helixcommunity.org;
> > > datatype-dev@helixcommunity.org; client-dev@lists.helixcommunity.org
> > > Subject: RE: [Client-dev] RE: [Filesystem-dev] Longer start-up time
> > > and buffer starvation on 420 Brizo & Head
> > > 
> > > What's 'HC CR'? If you mean the original change you referred, then the fact \
> > > that it tries to call FillBuffers from PacketReady means the original design \
> > > wanted to grab multiple packets within the FillBuffers/PacketReady loop. If it \
> > > makes we do one packet per ProcessIdle as of now, it's probably by accident, \
> > > not by design. 
> > > fxd
> > > 
> > > -----Original Message-----
> > > From: Junhong.Liu@nokia.com <Junhong.Liu@nokia.com>
> > > Sent: Wednesday, September 08, 2010 2:08 PM
> > > To: Sheldon Fu <sfu@real.com>
> > > Cc: Tony Seaward <tseaward@real.com>; filesystem-dev@helixcommunity.org \
> > > <filesystem-dev@helixcommunity.org>; datatype-dev@helixcommunity.org \
> > > <datatype-dev@helixcommunity.org>; client-dev@lists.helixcommunity.org \
> > >                 <client-dev@lists.helixcommunity.org>
> > > Subject: RE: [Client-dev] RE: [Filesystem-dev] Longer start-up time and buffer \
> > > starvation on 420        Brizo & Head 
> > > 
> > > In fact, the HC CR already made ProcessIdle to packet level.  Moving packet \
> > > away from the packet queue is not enough, we have to check preroll and other \
> > > status.  If we add all of these logic, basically it goes back to 210 CayS \
> > > architecture. 
> > > Regards,
> > > 
> > > Junhong
> > > 
> > > -----Original Message-----
> > > From: ext Sheldon Fu [mailto:sfu@real.com]
> > > Sent: Wednesday, September 08, 2010 12:56 PM
> > > To: Liu Junhong (Nokia-MS/Dallas)
> > > Cc: Tony Seaward; filesystem-dev@helixcommunity.org;
> > > datatype-dev@helixcommunity.org; client-dev@lists.helixcommunity.org
> > > Subject: Re: [Client-dev] RE: [Filesystem-dev] Longer start-up time
> > > and buffer starvation on 420 Brizo & Head
> > > 
> > > Calling ProcessIdle for each PacketReady probably isn't a good idea.
> > > ProcessIdle is a length function and it doesn't need packet level granularity I \
> > > think. 
> > > If moving the packet away from the packet queue is needed, we
> > > probably should move the loop in FillBuffers() and PacketReady higher
> > > up somewhere so it could include moving the packet away from the
> > > queue too (but not as high up as ProcessIdle)
> > > 
> > > fxd
> > > 
> > > Junhong.Liu@nokia.com wrote:
> > > 
> > > 
> > > 
> > > > Hi, Tony:
> > > > 
> > > > Thanks!  As we are using asynchronous read for Symbian ^3 and ^4,  I
> > > > figured out a possible solution for it as below.
> > > > 
> > > > --- e:\10.1_SDK\sf\mw\helix\helix_ren\src\client\core\hxflsrc.cpp
> > > > Wed Feb 24 18:43:10 2010
> > > > +++ hxflsrc.cpp Wed Sep 08 12:06:21 2010
> > > > *
> > > > HXFileSource::PacketReady(HX_RESULT status, IHXPacket* pPacket)
> > > > 
> > > > @@ -2800,9 +2804,8 @@
> > > > 
> > > > if(!m_bInFillMode)
> > > > {
> > > > -        FillBuffers(HX_GET_BETTERTICKCOUNT());
> > > > +       m_pPlayer->ProcessIdle(FALSE);
> > > > }
> > > > 
> > > > Because of the current FillBuffer() logic, in PacketReady()
> > > > callback, even call FillBuffer(), it won't issue GetPacket() again
> > > > since the streamInfo queue is not empty.  But if we call
> > > > HXPlayer::ProcessIdle() in PacketReady callback, the packet will be
> > > > moved to Renderer first and FillBuffer() inside ProcessIdle() will issue \
> > > > another GetPacket(). Another good thing for this change is that \
> > > > HXPlayer::ProcessIdle() also check the prerolling and other status, so it \
> > > > won't read more data than necessary.
> > > > 
> > > > I briefly tested it, the preroll can be shorten in half and no more
> > > > buffer starvation.  Compared with 210 CayS, the only drawback is
> > > > every packet reading will exercise ProcessIdle() once.  How do you
> > > > like this change?
> > > > 
> > > > After this change, there are (at least) three triggers for
> > > > ProcessIdle.  1. Scheduler timer  2. Asynchronous Read Done
> > > > (PacketReady)  3. Audio Device's OnTimeSync() report.  If we think
> > > > there are some overlapping between them and not good for CPU usage,
> > > > we can increase the Scheduler timer's intervals, and let PacketReady or
> > > > audio frame played out events to drive ProcessIdle.   I think it's
> > > > more efficient. How do you think?
> > > > 
> > > > Thanks,
> > > > 
> > > > Junhong
> > > > 
> > > > 
> > > > --------------------------------------------------------------------
> > > > -
> > > > -
> > > > --
> > > > *From:* ext Tony Seaward [mailto:tseaward@real.com]
> > > > *Sent:* Wednesday, September 08, 2010 11:30 AM
> > > > *To:* Liu Junhong (Nokia-MS/Dallas)
> > > > *Cc:* client-dev@lists.helixcommunity.org;
> > > > datatype-dev@helixcommunity.org; filesystem-dev@helixcommunity.org
> > > > *Subject:* Re: [Filesystem-dev] Longer start-up time and buffer
> > > > starvation on 420 Brizo & Head
> > > > 
> > > > Junhong-
> > > > I wanted to touch base and just let you know that we are looking
> > > > into this and should hopefully have some helpful response today or tomorrow.
> > > > Tony Seaward
> > > > Program Manager
> > > > RealNetworks
> > > > 
> > > > On 9/6/10 5:46 PM, Junhong.Liu@nokia.com wrote:
> > > > 
> > > > 
> > > > 
> > > > > Hi,
> > > > > 
> > > > > We moved to 420 Brizo branch for Symbian^4 platform.  However,
> > > > > compared with 210 CayS, we found it has longer start-up time and
> > > > > sometimes video renderer get starved for high frame rate clips.  We
> > > > > think these changes were introduced by this CR.
> > > > > 
> > > > > _http://lists.helixcommunity.org/pipermail/helix-client-dev/2006-Au
> > > > > g
> > > > > u
> > > > > st/004927.html_
> > > > > 
> > > > > In the above CR, it has changed the FillBuffer() logic from "read
> > > > > from the file to keep all streams filled their preroll value" to
> > > > > "Fill all stream buffer at least with one packet". After this
> > > > > change, in each Process Idle/FillBuffer(), at most it can only read one
> > > > > packet.   On Symbian Platform, the Process idle's timer is 30 ms.  So,
> > > > > 
> > > > > 1) Buffering phase:
> > > > > For a 30fps clip, if the preroll is 1 second, it will take 30 * 30
> > > > > ms = 900 ms to reach the preroll level.  On 210 CayS, the same clip
> > > > > only needs 100ms to read all preroll data.  Although this CR moves
> > > > > packets to Renderer and let them decoded/primed during the
> > > > > buffering phase, with TURBOPLAY feature off, renderers have to wait
> > > > > until HXPlayer changes state to Playing state before output these frames.
> > > > > So for users, the effective start-up time is 900ms + , which is
> > > > > longer than 210 CayS.
> > > > > 
> > > > > 2) Playing phase:
> > > > > During playing, because the max packet delivery rate to renderer is
> > > > > 33 fps (1000/30ms), it can not play higher frame rate clips.  And
> > > > > even for some lower frame rate clips, if for some reasons, one
> > > > > Process Idle is delayed, then the reading / delivering packet will
> > > > > be delayed too. And this delay can not be offset out, it will be
> > > > > accumulated until a re-buffering.  We have seen delivering buffer
> > > > > to renderer is getting slower and slower until a rebuffering.  And
> > > > > this cycle keeps repeating,
> > > > > 
> > > > > 3) Power save:
> > > > > In Power Save mode, when the audio pushdown reaches the low
> > > > > watermark, it resumes the scheduler  to read more packets until the
> > > > > high watermark.  Because helix core can only read one packet in one
> > > > > Process idle, it takes much longer to reach the high watermark. So
> > > > > the CPU idle time is much less than 210 CayS' same Power Save
> > > > > implementation.
> > > > > 
> > > > > Any suggestions on these issues?  We don't want to shorten the
> > > > > Process Idle's timer since it will unnecessarily increase the CPU
> > > > > usage.
> > > > > 
> > > > > Thanks,
> > > > > 
> > > > > Junhong
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > ---------------------------------------------------------------------
> > > -
> > > --
> > > 
> > > Subject:
> > > RE: [Client-dev] CR: JLIU-82PRPT: Time taken for preroll of local
> > > content is high with async data source
> > > From:
> > > "ext-asheesh.srivastava@nokia.com" <ext-asheesh.srivastava@nokia.com>
> > > Date:
> > > Tue, 16 Feb 2010 11:54:57 -0800
> > > To:
> > > Gregory Wright <gwright@real.com>
> > > 
> > > To:
> > > Gregory Wright <gwright@real.com>
> > > CC:
> > > "client-dev@helixcommunity.org" <client-dev@helixcommunity.org>
> > > 
> > > 
> > > Thanks Greg, changes committed to Brizo420 and HEAD.
> > > 
> > > Regards,
> > > - Asheesh
> > > 
> > > -----Original Message-----
> > > From: ext Gregory Wright [mailto:gwright@real.com]
> > > Sent: Tuesday, February 16, 2010 1:37 PM
> > > To: Srivastava Asheesh (EXT-InfoVision/Dallas)
> > > Cc: client-dev@helixcommunity.org
> > > Subject: Re: [Client-dev] CR: JLIU-82PRPT: Time taken for preroll of
> > > local content is high with async data source
> > > 
> > > Looks good.
> > > --greg.
> > > 
> > > 
> > > On Feb 16, 2010, at 7:06 AM, <ext-asheesh.srivastava@nokia.com> \
> > > <ext-asheesh.srivastava@nokia.com> wrote: 
> > > 
> > > 
> > > 
> > > > Hi,
> > > > 
> > > > Here is the fix merge for the following CR for Brizo_420 & HEAD.
> > > > 
> > > > "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:  ext-asheesh.srivastava@nokia.com
> > > > 
> > > > Reviewed by:
> > > > 
> > > > Date: 15-Feb-2010
> > > > 
> > > > Project:  SymbianMmf
> > > > 
> > > > Error Id:  JLIU-82PRPT (Applicable to Brizo420)
> > > > 
> > > > Synopsis: Time taken for preroll of local content is high with async
> > > > data source
> > > > 
> > > > Overview:
> > > > The time taken to buffer the initial preroll amount of data has increased \
> > > > substantially when asynchronous data source was introduced. 
> > > > FileSource invokes getPacket on fileformat during the players::processidle. \
> > > > On asynchronous file i/o, the call to FF->GetPacket does not result in \
> > > > FileSource::packetready within the same callstack. The callback happens on a \
> > > > callback from the system. The subsequent call to get next packet is issued \
> > > > only during the next processidle of player which based on current granularity \
> > > > is 30 msec. So in worst case the fileformat is not doing any operation during \
> > > > 30msec time which could be effectively used for retrieving the next packets. 
> > > > 
> > > > Solution Proposal:
> > > > Invoke HXFileSource::FillBuffer from HXFileSource::PacketReady callback only \
> > > > when it is not in m_bInFillMode. The change causes the file source to \
> > > > requests for next packet ( based on the existing conditions of fillendtime, \
> > > > etc) instead of waiting for next processidle of HXPlayer. The change will \
> > > > impact only asynchrnous file i/o and not synchronous i/o. In case of \
> > > > synchronous reads, the m_bInFillMode will be TRUE and the call to FillBuffers \
> > > > will not be executed. 
> > > > Files removed: None
> > > > 
> > > > Files added:  None
> > > > 
> > > > Files modified:
> > > > client/core/hxflsrc.cpp
> > > > 
> > > > Image size and heap use impact:  None
> > > > 
> > > > Test case(s) added:  No
> > > > 
> > > > Memory leak check performed:  Yes. No new leaks introduced
> > > > 
> > > > Platforms and profiles build verified:
> > > > helix-client-s60-52-mmf-mdf-dsp
> > > > 
> > > > Platforms and profiles functionality verified:  armv5, winscw
> > > > 
> > > > Branch:  HEAD, 420Brizo
> > > > 
> > > > Diff:
> > > > 
> > > > Index: hxflsrc.cpp
> > > > ===================================================================
> > > > RCS file: /cvsroot/client/core/hxflsrc.cpp,v
> > > > retrieving revision 1.138.12.1
> > > > diff -u -b -r1.138.12.1 hxflsrc.cpp
> > > > --- hxflsrc.cpp 18 Sep 2009 04:46:03 -0000      1.138.12.1
> > > > +++ hxflsrc.cpp 15 Feb 2010 19:43:20 -0000
> > > > @@ -2798,6 +2798,11 @@
> > > > }
> > > > }
> > > > 
> > > > +    if(!m_bInFillMode)
> > > > +    {
> > > > +        FillBuffers(HX_GET_BETTERTICKCOUNT());
> > > > +    }
> > > > +
> > > > if (!theErr)
> > > > {
> > > > m_bReceivedData = TRUE;
> > > > @@ -3313,6 +3318,8 @@
> > > > return HXR_OK;
> > > > }
> > > > 
> > > > +    m_bInFillMode = TRUE;
> > > > +
> > > > // If we need to limit this processing and processing time allowance is not \
> > > > set, // acquire the processing time allowance from the player.
> > > > if ((ulLoopEntryTime != 0) && (ulProcessingTimeAllowance == 0))
> > > > @@
> > > > -3385,6 +3392,8 @@
> > > > m_pPlayer,
> > > > this);
> > > > 
> > > > +    m_bInFillMode = FALSE;
> > > > +
> > > > return retVal;
> > > > }
> > > > 
> > > > Thanks,
> > > > - Asheesh
> > > > 
> > > > 
> > > > 
> > > > 
> > > > -----Original Message-----
> > > > From: client-dev-bounces@helixcommunity.org
> > > > [mailto:client-dev-bounces@helixcommunity.org] On Behalf Of
> > > > Rathinasamy Rajesh (Nokia-D/Dallas)
> > > > Sent: Tuesday, January 12, 2010 10:33 AM
> > > > To: ehyche@real.com
> > > > Cc: client-dev@helixcommunity.org
> > > > Subject: RE: RESEND: RESEND: [Client-dev] CR: Time taken for preroll
> > > > of local content is high with async data source
> > > > 
> > > > Hi Eric,
> > > > Thanks for your time and comments. I have checked in the changes only to \
> > > > 210CayS, right now. The FillBuffer in Head and 420Brizo is slightly different \
> > > > and I don't want to check that in before testing. We will submit it later. 
> > > > Thanks,
> > > > Rajesh.
> > > > 
> > > > 
> > > > -----Original Message-----
> > > > From: ext Eric Hyche [mailto:ehyche@real.com]
> > > > Sent: Monday, January 11, 2010 9:38 AM
> > > > To: Rathinasamy Rajesh (Nokia-D/Dallas)
> > > > Cc: client-dev@helixcommunity.org
> > > > Subject: Re: RESEND: RESEND: [Client-dev] CR: Time taken for preroll
> > > > of local content is high with async data source
> > > > 
> > > > Rajesh,
> > > > 
> > > > I understand your explanation. This seems fine to me.
> > > > 
> > > > Eric
> > > > 
> > > > On Jan 8, 2010, at 4:42 PM, <rajesh.rathinasamy@nokia.com> \
> > > > <rajesh.rathinasamy@nokia.com> wrote: 
> > > > 
> > > > 
> > > > 
> > > > > Hi,
> > > > > Can someone else please review this CR ?
> > > > > 
> > > > > Thanks,
> > > > > Rajesh.
> > > > > 
> > > > > 
> > > > > -----Original Message-----
> > > > > From: Rathinasamy Rajesh (Nokia-D/Dallas)
> > > > > Sent: Thursday, January 07, 2010 5:27 PM
> > > > > To: Rathinasamy Rajesh (Nokia-D/Dallas); ehyche@real.com
> > > > > Cc: client-dev@helixcommunity.org
> > > > > Subject: RESEND: [Client-dev] CR: Time taken for preroll of local
> > > > > content is high with async data source
> > > > > 
> > > > > 
> > > > > 
> > > > > -----Original Message-----
> > > > > From: Rathinasamy Rajesh (Nokia-D/Dallas)
> > > > > Sent: Thursday, January 07, 2010 6:37 AM
> > > > > To: Rathinasamy Rajesh (Nokia-D/Dallas); ehyche@real.com
> > > > > Cc: client-dev@helixcommunity.org
> > > > > Subject: RE: [Client-dev] CR: Time taken for preroll of local
> > > > > content is high with async data source
> > > > > 
> > > > > Hi Eric,
> > > > > Not sure if my previous mail explained the problem clearly. IN short the \
> > > > > GetPacket call to Fileformat was invoked for every 30msec and the loading \
> > > > > time for a preroll time of 3.5 sec data (750kbps) was around 4 sec. 
> > > > > I did the testing with 3GP filformat and based on the analysis, the problem \
> > > > > should appear for all fileformats. 
> > > > > Thanks,
> > > > > Rajesh.
> > > > > 
> > > > > 
> > > > > -----Original Message-----
> > > > > From: client-dev-bounces@helixcommunity.org
> > > > > [mailto:client-dev-bounces@helixcommunity.org] On Behalf Of
> > > > > Rathinasamy Rajesh (Nokia-D/Dallas)
> > > > > Sent: Wednesday, January 06, 2010 9:08 AM
> > > > > To: ehyche@real.com
> > > > > Cc: client-dev@helixcommunity.org
> > > > > Subject: RE: [Client-dev] CR: Time taken for preroll of local
> > > > > content is high with async data source
> > > > > 
> > > > > Hi Eric,
> > > > > Thanks for a quick response. Comments inlined.
> > > > > 
> > > > > Thanks,
> > > > > Rajesh.
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: ext Eric Hyche [mailto:ehyche@real.com]
> > > > > > Sent: Wednesday, January 06, 2010 8:47 AM
> > > > > > To: Rathinasamy Rajesh (Nokia-D/Dallas)
> > > > > > Cc: client-dev@helixcommunity.org
> > > > > > Subject: Re: [Client-dev] CR: Time taken for preroll of local
> > > > > > content is high with async data source
> > > > > > 
> > > > > > Rajesh,
> > > > > > 
> > > > > > I'm a bit concerned about this change, because: a) this is very
> > > > > > sensitive code which has not been touched in a while; and
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > <<Rajesh-06Jan>> Sure. I agree that this code is sensitive and not being \
> > > > > touched for a while. But I'm not sure how much was it tested with \
> > > > > asnchronous file i/o. 
> > > > > 
> > > > > 
> > > > > 
> > > > > > b) even file formats using synchronous I/O file objects will not
> > > > > > always return a packet with PacketReady() every time they are
> > > > > > called with GetPacket(), particularly if they are doing merge
> > > > > > sorting between streams. I'm concerned that this could result us
> > > > > > being put in very tight, CPU-consuming loops.
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > <<Rajesh-06Jan>> In case of synchronous I/O file and packetReady coming \
> > > > > asynchronously, this change is only going to issue the next call to \
> > > > > getPacket based on the existing logic in FillBuffer which already seems to \
> > > > > have logic to addrress cases of calling from FillBuffer. If the callstack \
> > > > > is already in FillBuffer, then this call from PacketReady is not invoked. 
> > > > > 
> > > > > 
> > > > > 
> > > > > > Perhaps I don't understand the problem fully. It seems to me that
> > > > > > the asynchronous ReadDone() callback to the file format should
> > > > > > result (if a packet is completed and a GetPacket request is
> > > > > > outstanding) in a
> > > > > > PacketReady() callback to the file source. Is this not happening?
> > > > > > Which file format plugin are you seeing this with?
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > <<Rajesh-06Jan>> The problem is not in ReadDone ending up with PacketReady. \
> > > > > The problem is PacketReady is not ending up in request for next GetPacket \
> > > > > fast enough. Let me give an example of call flow with asynchronous \
> > > > > PacketReady. Call stack is not complete stack, just picked a few important \
> > > > > for this discussion. TimeStamp  Func Name
> > > > > =========  ==========
> > > > > 1000 HXPLayer::ProcessIdle() <-----IN 1000
> > > > > HXFileSource::FillBuffer() <----IN
> > > > > 1001 FileFormat::GetPacket(0)
> > > > > 1001 FileFormat::GetPacket(1)
> > > > > 1001 HXFileSource::FillBuffer ---->OUT
> > > > > 1005 HXPlayer::SchedulePLayer // Schedules for 30msec
> > > > > 1005 HXPlayer::ProcessIdle() ---->OUT
> > > > > 1006 HXFileSource::PacketReady(0)
> > > > > 1006 HXFileSource::PacketReady(1)
> > > > > 1035 HXPLayer::ProcessIdle() <-----IN
> > > > > 1035 HXFileSource::FillBuffer() <----IN
> > > > > 1036 FileFormat::GetPacket(0)
> > > > > 1036 FileFormat::GetPacket(1)
> > > > > 1037 HXFileSource::FillBuffer ---->OUT
> > > > > 1038 HXPlayer::SchedulePLayer // Schedules for 30msec
> > > > > 1038 HXPlayer::ProcessIdle() ---->OUT
> > > > > 
> > > > > If you notice the GetPacket call to fileformat, it happens only during the \
> > > > > next processidle of hxplayer. During this gap, the fileformat, Source, \
> > > > > fileobject are not doing anything. 
> > > > > In case of PacketReady being returned synchronously within the same \
> > > > > callstack, this is exactly what is happening..A next GetPacket call is \
> > > > > issued right away. 
> > > > > 
> > > > > 
> > > > > 
> > > > > > Eric
> > > > > > 
> > > > > > On Jan 5, 2010, at 6:24 PM, <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: Ashish.As.Gupta@nokia.com
> > > > > > > 
> > > > > > > Date: 05-Jan-2010
> > > > > > > 
> > > > > > > Project:  SymbianMmf
> > > > > > > 
> > > > > > > Error Id:  DALM-7ZEU4L
> > > > > > > 
> > > > > > > Synopsis: Time taken for preroll of local content is high with
> > > > > > > async data source
> > > > > > > 
> > > > > > > Overview:
> > > > > > > The time taken to buffer the initial preroll amount of
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > data has increased substantially when asynchronous data source was
> > > > > > introduced.
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > FileSource invokes getPacket on fileformat during the
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > players::processidle. On asynchronous file i/o, the call to
> > > > > > FF->GetPacket does not result in FileSource::packetready
> > > > > > within the same callstack. The callback happens on a callback from
> > > > > > the system. The subsequent call to get next packet is issued only
> > > > > > during the next processidle of player which based on current
> > > > > > granularity is 30 msec. So in worst case the fileformat is not
> > > > > > doing any operation during 30msec time which could be effectively
> > > > > > used for retrieving the next packets.
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > Solution Proposal:
> > > > > > > Invoke HXFileSource::FillBuffer from
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > HXFileSource::PacketReady callback only when it is not in
> > > > > > m_bInFillMode. The change causes the file source to requests for
> > > > > > next packet ( based on the existing conditions of fillendtime,
> > > > > > etc) instead of waiting for next processidle of HXPlayer. The
> > > > > > change will impact only asynchrnous file i/o and not synchronous
> > > > > > i/o. In case of synchronous reads, the m_bInFillMode will be TRUE
> > > > > > and the call to FillBuffers will not be executed.
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > > Files removed: None
> > > > > > > 
> > > > > > > Files added:  None
> > > > > > > 
> > > > > > > Files modified:
> > > > > > > client/core/hxflsrc.cpp
> > > > > > > 
> > > > > > > Image size and heap use impact:  None
> > > > > > > 
> > > > > > > Module release testing (STIF):  PASSED
> > > > > > > 
> > > > > > > Test case(s) added:  No
> > > > > > > 
> > > > > > > Memory leak check performed:  Yes. No new leaks introduced
> > > > > > > 
> > > > > > > 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: hxflsrc.cpp
> > > > > > > =================================================================
> > > > > > > = = RCS file: /cvsroot/client/core/hxflsrc.cpp,v
> > > > > > > retrieving revision 1.96.2.14
> > > > > > > diff -w -u -b -r1.96.2.14 hxflsrc.cpp
> > > > > > > --- hxflsrc.cpp 25 Mar 2009 16:34:36 -0000      1.96.2.14
> > > > > > > +++ hxflsrc.cpp 5 Jan 2010 23:04:27 -0000
> > > > > > > @@ -2611,6 +2611,10 @@
> > > > > > > }
> > > > > > > }
> > > > > > > }
> > > > > > > +    else
> > > > > > > +    {
> > > > > > > +        FillBuffers();
> > > > > > > +    }
> > > > > > > 
> > > > > > > if (!theErr)
> > > > > > > {
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > <ATT00001..txt>
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > Eric Hyche (ehyche@real.com)
> > > > > > Principal Engineer
> > > > > > RealNetworks, Inc.
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > _______________________________________________
> > > > > Client-dev mailing list
> > > > > Client-dev@helixcommunity.org
> > > > > http://lists.helixcommunity.org/mailman/listinfo/client-dev
> > > > > 
> > > > > 
> > > > > 
> > > > Eric Hyche (ehyche@real.com)
> > > > Principal Engineer
> > > > RealNetworks, Inc.
> > > > 
> > > > 
> > > > 
> > > > _______________________________________________
> > > > Client-dev mailing list
> > > > Client-dev@helixcommunity.org
> > > > http://lists.helixcommunity.org/mailman/listinfo/client-dev
> > > > 
> > > > 
> > > > _______________________________________________
> > > > Client-dev mailing list
> > > > Client-dev@helixcommunity.org
> > > > http://lists.helixcommunity.org/mailman/listinfo/client-dev
> > > > 
> > > > 
> > > > 
> > > _______________________________________________
> > > Client-dev mailing list
> > > Client-dev@helixcommunity.org
> > > http://lists.helixcommunity.org/mailman/listinfo/client-dev
> > > 
> > > 
> > > 
> > .
> > 
> > 
> > 
> 
> 
> .
> 
> 



_______________________________________________
Filesystem-dev mailing list
Filesystem-dev@helixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/filesystem-dev


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

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