Problems with byte swapping in audio code

Dave Chapman <[email protected]> Fri, 11 Apr 2003 11:20:17 +0100
Newsgroups gmane.comp.video.ogle.devel
Message-ID <[email protected]>
Björn,

I am attempting to add 24-bit/96KHz LPCM audio support to ogle and the 
latest version of my patch is included at the end of this mail (are 
attachments allowed on this list?).

My current approach is to simply make the oss_audio.c driver accept 
requests by ogle for 24-bit/96KHz audio, and do the software conversion 
to 16-bit/48KHz inside the OSS driver code.  I've also had to make some 
tiny changes to ogle so it correctly passes 24-bit data to the audio 
drivers.

I will eventually do things in a better way (probing for audio device 
capabilities and converting the audio inside ogle), but the attached 
patch works.  However, I am confused about why it is working!.

Also, I think that I can confirm that 24-bit audio is stored as 3 bytes 
per sample in the stream (not 4 bytes as the comment in decode_lpcm.c 
states).

My code (in oss_play) to convert from 24 to 16 bits currently looks like 
this:

char data[4];

for (i=0;i<nbyte;i+=6) {
   // Left channel
   data[0]=((char*)samples)[i+1];
   data[1]=((char*)samples)[i];
   // Right Channel
   data[2]=((char*)samples)[i+5];
   data[3]=((char*)samples)[i+2];
   write(instance->fd,data,4);
}

I have tried many different versions of the above code - but the one I 
give here is the only one that is giving me the correct stereo sound. 
This was found by trial-and-error.

This may look strange, but I think that it can be explained if there is 
  byte-swapping going on in the 3 words:

i.e. the data for two 24-bit samples starts on the DVD as 0,1,2,3,4,5 
where (0,1,2) is the first sample, and (3,4,5) is the second.

But I think that it gets swapped to 1,0,3,2,5,4

So to take the original most significant 16 bits, I need bytes 0 and 1 
and 3 and 4 from the original order.

This translates to bytes 0,1 and bytes 2,5 in the "swapped" layout - 
which is what my code is doing.

Does this make any sense to you?  Can you identify where this 
byte-swapping is happenning?  I am using an Intel Pentium-3 machine for 
my testing.

Grateful for any pointers.

Regards,

Dave.


diff -u3 -r ogle-0.9.1/ac3/conversion.c ogle-0.9.1-24bit/ac3/conversion.c
--- ogle-0.9.1/ac3/conversion.c	Thu Jan  2 22:06:28 2003
+++ ogle-0.9.1-24bit/ac3/conversion.c	Fri Apr 11 09:43:22 2003
@@ -130,7 +130,18 @@
      conversion_routine = 0;
      break;
    case SampleFormat_Signed:
-    conversion_routine = 1;
+    switch(src_format->sample_size) {
+    case 2:
+      conversion_routine = 1;
+      break;
+    case 3:
+      conversion_routine = 5;
+      break;
+    default:
+      FATAL("init_conversion: LPCM SampleSize %d not supported\n",
+	     src_format->sample_size);
+      break;
+    }
      break;
    case SampleFormat_MadFixed:
      conversion_routine = 2;
@@ -406,6 +417,10 @@
  				      nr_samples);
      h->output_buf_ptr += 2*2*nr_samples; // 2ch 16bit 48kHz

+    break;
+  case 5:
+    memcpy(h->output_buf_ptr,samples,nr_samples * 2 * 3);
+    h->output_buf_ptr += 2*3*nr_samples;
      break;
    }
     return 0;
diff -u3 -r ogle-0.9.1/ac3/decode_lpcm.c ogle-0.9.1-24bit/ac3/decode_lpcm.c
--- ogle-0.9.1/ac3/decode_lpcm.c	Mon Dec 16 17:10:22 2002
+++ ogle-0.9.1-24bit/ac3/decode_lpcm.c	Fri Apr 11 10:04:15 2003
@@ -119,7 +119,7 @@
        break;
      case 2:
        new_quantization_word_length = 24;
-      new_sample_size = 4; // ? 24bit in 32bit ?
+      new_sample_size = 3; // Is it 3 or 4 bytes?  I'm still not sure.
        break;
      default:
        new_sample_size = 0;
diff -u3 -r ogle-0.9.1/libogleao/oss_audio.c 
ogle-0.9.1-24bit/libogleao/oss_audio.c
--- ogle-0.9.1/libogleao/oss_audio.c	Wed Nov 20 21:14:58 2002
+++ ogle-0.9.1-24bit/libogleao/oss_audio.c	Fri Apr 11 10:58:59 2003
@@ -49,6 +49,7 @@
    int channels;
    int speed;
    int initialized;
+  int half_speed;
  } oss_instance_t;

  static int log2(int val)
@@ -114,6 +115,7 @@
    switch(audio_info->encoding) {
    case OGLE_AO_ENCODING_LINEAR:
      switch(audio_info->sample_resolution) {
+      case 24:
        case 16:
  	if(audio_info->byteorder == OGLE_AO_BYTEORDER_BE) {
  	  sample_format = AFMT_S16_BE;
@@ -247,14 +249,23 @@
      audio_info->sample_rate = -1;
      return -1;
    }
+  if (audio_info->sample_rate != sample_speed) {
+    if (audio_info->sample_rate==(2 * sample_speed)) {
+      sample_speed=audio_info->sample_rate;
+      instance->half_speed=1;
+    }
+  } else {
+    instance->half_speed=0;
+  }
    instance->speed = sample_speed;
    // report back the actual speed used
    audio_info->sample_rate = sample_speed;

    single_sample_size = (audio_info->sample_resolution + 7) / 8;
-  if(single_sample_size > 2) {
-    single_sample_size = 4;
-  }
+  // ?? I think 24-bit audio is in 3 bytes, but I'm not sure yet.
+  //  if(single_sample_size > 2) {
+  //    single_sample_size = 4;
+  //  }
    instance->sample_frame_size = single_sample_size*number_of_channels;

    audio_info->sample_frame_size = instance->sample_frame_size;
@@ -319,6 +330,15 @@
      perror("SNDCTL_DSP_SPEED");
      return -1;
    }
+  /* We need to maintain the "half_speed" flag in reinit */
+  if (instance->speed != sample_speed) {
+    if (instance->speed==(2 * sample_speed)) {
+      sample_speed=instance->speed;
+      instance->half_speed=1;
+    }
+  } else {
+    instance->half_speed=0;
+  }
    if(sample_speed != instance->speed) {
      fprintf(stderr, "oss_audio: couldn't reinit speed\n");
      return -1;
@@ -344,11 +364,31 @@
  {
    oss_instance_t *instance = (oss_instance_t *)_instance;
    int written;
-
-  written = write(instance->fd, samples, nbyte);
-  if(written == -1) {
-    perror("audio write");
-    return -1;
+  int i;
+  char data[4];
+
+  // To do - move these conversions to ac3/conversion.c
+  if (instance->sample_frame_size==6) {
+    written=0;
+    for (i=0;i<nbyte;i+=6) {
+        // Why is it bytes 1 and 2 for left channel, and bytes 2 and 3 for
+        // the right channel?  Has byte-swapping within the 3 words 
happened?
+        data[0]=((char*)samples)[i+1];
+        data[1]=((char*)samples)[i];
+        data[2]=((char*)samples)[i+5];
+        data[3]=((char*)samples)[i+2];
+        write(instance->fd,data,4);
+        if(instance->half_speed) {
+          i+=6;
+        }
+    }
+  } else {
+    /* TO DO: Implement "half_speed" for 16 bit audio */
+    written = write(instance->fd, samples, nbyte);
+    if(written == -1) {
+      perror("audio write");
+      return -1;
+    }
    }

    return 0;