Looks good for checkin. ======================================= Eric Hyche (ehy...@real.com) Principal Engineer RealNetworks, Inc.
>-----Original Message----- >From: Sheldon fu [mailto:s...@real.com] >Sent: Friday, May 01, 2009 10:01 AM >To: ehy...@real.com >Cc: audio-dev@helixcommunity.org >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 (ehy...@real.com) >> Principal Engineer >> RealNetworks, Inc. >> >> >> >-----Original Message----- >> >From: audio-dev-boun...@helixcommunity.org >> >[mailto:audio-dev-boun...@helixcommunity.org] On Behalf >Of >> >Sheldon fu >> >Sent: Wednesday, April 29, 2009 11:14 AM >> >To: audio-dev@helixcommunity.org >> >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 Audio-dev@helixcommunity.org http://lists.helixcommunity.org/mailman/listinfo/audio-dev
