Checked into Head, 310 and 349. What is 349atlas branch for? Should optimization-related change to 310 always be checked into 349 too?
fxd On Fri, 2009-05-01 at 08:04 -0700, Rishi Mathew wrote: > Also to 349atlas plz. > > Rishi Mathew > - Sent via mobile phone > > -----Original Message----- > From: Eric Hyche <[email protected]> > Sent: Friday, May 01, 2009 7:07 AM > To: 'Sheldon fu' <[email protected]> > Cc: [email protected] > Subject: RE: [Audio-dev] CR: Eliminate memory copying in mix engine for > single player/stream case > > Looks good for checkin. > > ======================================= > Eric Hyche ([email protected]) > Principal Engineer > RealNetworks, Inc. > > > >-----Original Message----- > >From: Sheldon fu [mailto:[email protected]] > >Sent: Friday, May 01, 2009 10:01 AM > >To: [email protected] > >Cc: [email protected] > >Subject: RE: [Audio-dev] CR: Eliminate memory copying in mix engine for > >single player/stream case > > > >Thanks Eric. Good advice. Here is the safe-guard in unix audio device > >code to error out when both features are defined. MIN_ROLLBACK_BUFFER is > >a local define in audio device so the check can not be in the session > >object itself. > > > >MIN_ROLLBACK_BUFFER is defined when MIN_HEAP is used so this pretty much > >makes OPTIMIZED_MIXING not usable for any Unix build with MIN_HEAP. Once > >this CR is checked in I'll have a look to see if the MIN_ROLLBACK_BUFFER > >logic can be changed so it doesn't make the assumption that we always > >call Write() with a whole block of data. > > > >I'll check in the change EOD if there is no further comment. > > > >Thanks! > > > >fxd > > > > > >Index: platform/unix/audUnix.cpp > >=================================================================== > >RCS file: /cvsroot/audio/device/platform/unix/audUnix.cpp,v > >retrieving revision 1.12.2.4 > >diff -u -w -r1.12.2.4 audUnix.cpp > >--- platform/unix/audUnix.cpp 17 Mar 2009 12:46:53 -0000 1.12.2.4 > >+++ platform/unix/audUnix.cpp 1 May 2009 12:52:02 -0000 > >@@ -989,6 +989,20 @@ > > // In heap-optimized mode, this is where we set the value of > > // m_ulDeviceBufferSize, and malloc the buffer. > > #ifdef HELIX_CONFIG_MIN_ROLLBACK_BUFFER > >+ > >+ //OPTIMIZED_MIXING conflicts with MIN_ROLLBACK_BUFFER feature. > >+ //This is because the logic here assumes that the audio device > >+ //Write() method is always called with a constant size -- the > >block size, > >+ //while in OPTIMIZED_MIXING mode, session will write partial > >blocks to avoid > >+ ///memory copying. > >+ //TODO: MIN_ROLLBACK_BUFFER logic should be improved so that it > >gets > >+ //the block size from audio session through a new API and > >reduce the rollback size > >+ //to the block size, rather than second guessing it from > >Write() call here. > >+ //Error out for now if both features are defined. > >+#ifdef HELIX_FEATURE_OPTIMIZED_MIXING > >+#error "Can not have both HELIX_FEATURE_OPTIMIZED_MIXING and > >HELIX_CONFIG_MIN_ROLLBACK_BUFFER" > >+#endif > >+ > > if( m_ulDeviceBufferSize != nBuffSize ) > > { > > m_ulDeviceBufferSize = nBuffSize; > > > > > >On Fri, 2009-05-01 at 09:13 -0400, Eric Hyche wrote: > >> This change looks good. > >> > >> >Also this feature conflicts with HELIX_CONFIG_MIN_ROLLBACK_BUFFER that > >> >the unix audio device uses. > >> > >> Can you make an intentional preprocessor error (#error) if hxaudses.cpp > >> (or whatever file is > >appropriate) is compiled with these > >> conflicting features. A compile-time error will be much easier to catch > >> than a run-time error. > >> > >> Eric > >> > >> ======================================= > >> Eric Hyche ([email protected]) > >> Principal Engineer > >> RealNetworks, Inc. > >> > >> > >> >-----Original Message----- > >> >From: [email protected] > >> >[mailto:[email protected]] On Behalf > >Of > >> >Sheldon fu > >> >Sent: Wednesday, April 29, 2009 11:14 AM > >> >To: [email protected] > >> >Subject: [Audio-dev] CR: Eliminate memory copying in mix engine for > >> >single player/stream case > >> > > >> >Synopsis: > >> > > >> >This change gets rid of unnecessary memcpy and/or equivalent when mixing > >> >is not really needed. > >> > > >> > > >> >Overview: > >> > > >> >Currently even when there is only a single player/stream, we still run > >> >PCM data through the DSP mix > >> >engine. The mix engine logic will do two memory copying operations even > >> >when in/out format matches > >> >(same sample rate, channels and bytesPerSample) -- one in > >> >ConvertIntoBuffer when it asks for the > >data > >> >from stream object and one to mix the data into the output buffer session > >> >object provides. These > >two > >> >memcpys are pure overhead in the typical single player/stream playback > >> >case. > >> > > >> >The change adds a buffer list parameter to the stream object > >> >MixIntoBuffer and in turn mixengine > >> >MixIntoBuffer method and a flag to control it. When both session and > >> >mixengine agrees to do the > >> >'bypass' > >> >optimization, the stream object will return a list of buffers containing > >> >exactly one block of audio > >> >data if put together. Stream object constructs this list from its > >> >internal data buffers with > >> >CHXBufferFragment to avoid create any new memory buffer. mix engine > >> >MixIntoBuffer still runs > >through > >> >its logic but it will not touch any actual data (no ConvertIntoBuffer > >> >call and no mixing into > >output > >> >buffer). Stream object sends the buffer in the 'direct list' to the audio > >> >device without change. > >> > > >> >Session object agress to do 'bypass' when it knows there is only a single > >> >player/stream and when > >the > >> >new HELIX_FEATURE_OPTIMIZED_MIXING is defined. > >> > > >> >MixEngine agrees to do 'bypass' when it knows that there is nothing it > >> >really needs to do -- same > >> >in/out format and when some of the DSP features are not defined. The > >> >logic here can be improved in > >the > >> >future so that we can handle these features dynamically -- e.g. when > >> >cross fade is on, we can still > >do > >> >bypass until to the point that cross fade kicks in. This is not coded yet > >> >since this work is mainly > >> >for optimization on Android and Android build doesn't have these DSP > >> >features defined currently. > >> > > >> >Also this feature conflicts with HELIX_CONFIG_MIN_ROLLBACK_BUFFER that > >> >the unix audio device uses. > >> >When HELIX_CONFIG_MIN_ROLLBACK_BUFFER is on, unix audio device > >> >dynamically adjust its rollback > >buffer > >> >size to the incoming buffer size every time _Write is called. That > >> >apparently only can work when > >> >_Write is always called with a constant buffer size and if not, it will > >> >break. Not sure why we have > >> >such logic there. Also the rollback handling in unix audio device makes > >> >copies of incoming data > >too. > >> > > >> >Even with the change, the 'artificial' block-based handling of audio data > >> >is still not efficient. > >> >Though now there is no memcpy anymore, the control logic is quite > >> >complicated. OS Audio API can > >handle > >> >any size write, up to the maximum buffer limit and for the simplest case, > >> >we should just write the > >> >decoded PCM frame data to OS audio device in a > >> >1-to-1 mapping way. > >> > > >> >On Android, the change results in about 1-2% CPU usage saving when > >> >playing a MP3 clip and there is > >no > >> >noticeable change on my host Linux box (the machine is too fast already I > >> >think). > >> > > >> >This CR also contains the change in the previous CR for mixengine > >> >cvt16/cvt32 optimization. > >> > > >> >Head and Atlas310. > >> > > >> >fxd > >> > >> > > > _______________________________________________ > Audio-dev mailing list > [email protected] > http://lists.helixcommunity.org/mailman/listinfo/audio-dev > _______________________________________________ Audio-dev mailing list [email protected] http://lists.helixcommunity.org/mailman/listinfo/audio-dev
