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