ALSA capture corruption when using SND_PCM_FORMAT_S16_LE fix

Linux
1

I’ve noticed there is a problem with ALSA capture when using SND_PCM_FORMAT_S16_LE; in fact, there is a problem when using any format where sizeof(format) != sizeof(float).

In juce_linux_Audio.cpp, in the ALSADevice class read and write methods, you are calling convertFloatToFormat and convertFormatToFloat, but providing the same pointer for both destination and source. Because these methods both convert ‘in-place’, they are only safe to call when both destination and source formats have identical sizes.

In the case with S16_LE (little endian, short), the first sample frame short is read, converted to float, and then written back to the same destination. Which, due to the fact that float is 4 bytes, will overwrite the second sample frame. Every other sample is therefore corrupted.

Now, I have a fix in the ALSADevice class; basically, it uses 2 scratch classes so conversions are no longer in-place. Caveat: All sound cards I’ve tried this on only seem to support interleaved S16, so I’ve yet to try it with any other formats. Below is a diff of the changes; I can supply the raw file if you’d prefer that.

--- ../../juce20081020/build/linux/platform_specific_code/juce_linux_Audio.cpp	1984-09-07 10:12:32.000000000 +0100
+++ ../juce/build/linux/platform_specific_code/juce_linux_Audio.cpp	2008-11-03 12:52:05.000000000 +0000
@@ -265,11 +265,13 @@
     {
         if (isInterleaved)
         {
-            scratch.ensureSize (sizeof (float) * numSamples * numChannelsRunning, false);
+            scratch.ensureSize((bitDepth>>3) * numSamples * numChannelsRunning, false);
+            scratchFloat.ensureSize(sizeof (float) * numSamples * numChannelsRunning, false);
             float* interleaved = (float*) scratch;
+            float* interleavedFloat = (float*)scratchFloat;
 
-            AudioDataConverters::interleaveSamples ((const float**) data, interleaved, numSamples, numChannelsRunning);
-            AudioDataConverters::convertFloatToFormat (sampleFormat, interleaved, interleaved, numSamples * numChannelsRunning);
+            AudioDataConverters::interleaveSamples ((const float**) data, interleavedFloat, numSamples, numChannelsRunning);
+            AudioDataConverters::convertFloatToFormat (sampleFormat, interleavedFloat, interleaved, numSamples * numChannelsRunning);
 
             snd_pcm_sframes_t num = snd_pcm_writei (handle, (void*) interleaved, numSamples);
 
@@ -280,8 +282,11 @@
         {
             for (int i = 0; i < numChannelsRunning; ++i)
                 if (data[i] != 0)
-                    AudioDataConverters::convertFloatToFormat (sampleFormat, data[i], data[i], numSamples);
-
+                {
+                    scratchFloat.ensureSize(sizeof(float) * numSamples);
+                    scratchFloat.copyFrom(data[i], 0, sizeof(float) * numSamples); 		
+                    AudioDataConverters::convertFloatToFormat (sampleFormat, (float*)scratchFloat, data[i], numSamples);
+                }
             snd_pcm_sframes_t num = snd_pcm_writen (handle, (void**) data, numSamples);
 
             if (failed (num))
@@ -303,8 +308,10 @@
     {
         if (isInterleaved)
         {
-            scratch.ensureSize (sizeof (float) * numSamples * numChannelsRunning, false);
+            scratch.ensureSize((bitDepth>>3) * numSamples * numChannelsRunning, false);
+            scratchFloat.ensureSize(sizeof (float) * numSamples * numChannelsRunning, false);
             float* interleaved = (float*) scratch;
+            float* interleavedFloat = (float*)scratchFloat;
 
             snd_pcm_sframes_t num = snd_pcm_readi (handle, (void*) interleaved, numSamples);
 
@@ -319,8 +326,8 @@
                     return false;
             }
 
-            AudioDataConverters::convertFormatToFloat (sampleFormat, interleaved, interleaved, numSamples * numChannelsRunning);
-            AudioDataConverters::deinterleaveSamples (interleaved, data, numSamples, numChannelsRunning);
+            AudioDataConverters::convertFormatToFloat (sampleFormat, interleaved, interleavedFloat, numSamples * numChannelsRunning);
+            AudioDataConverters::deinterleaveSamples (interleavedFloat, data, numSamples, numChannelsRunning);
         }
         else
         {
@@ -331,7 +338,11 @@
 
             for (int i = 0; i < numChannelsRunning; ++i)
                 if (data[i] != 0)
-                    AudioDataConverters::convertFormatToFloat (sampleFormat, data[i], data[i], numSamples);
+                {
+                    scratch.ensureSize((bitDepth>>3) * numSamples);
+                    scratch.copyFrom(data[i], 0, (bitDepth>>3) * numSamples); 		
+                    AudioDataConverters::convertFormatToFloat (sampleFormat, (float*)scratch, data[i], numSamples);
+                }
         }
 
         return true;
@@ -349,6 +360,7 @@
     const bool isInput;
     bool isInterleaved;
     MemoryBlock scratch;
+    MemoryBlock scratchFloat;
     AudioDataConverters::DataFormat sampleFormat;
 
     //==============================================================================

Additionally, would it be worth adding an assert to the AudioDataConversions class to warn against in-place conversions of different sized types? In fact, perhaps that would be a neater solution: amend the AudioDataConversions class so that when conversions of different size types with the same source and destination pointer are required, the source is first copied to a temporary MemoryBlock?

2

Thanks for reporting that one. I think the best solution is actually simpler - if a few of the data converter ops are rewritten to run backwards, they should work correctly on in-place data. Have a go with the version of juce_AudioDataConverters that I’ve just checked in and see if that sorts you out…

3

I’ve just tried the fix you made to the AudioDataConverters class; works great - cheers!