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

Reply via email to