oggdec channel ordering bug

Nicolas George <[email protected]> Wed, 4 Jul 2012 12:18:12 +0200
Newsgroups gmane.comp.multimedia.ogg.vorbis.devel
Message-ID <[email protected]> 
Hi.

I just noticed that oggdec channel reordering for WAV is bogus.

With irrelevant clutter removed, oggenc has the following code:

	permute = {0,2,1,4,5,3}                 // oggenc/audio.c:401
	for (j)
	    to_vorbis[j] = from_wav[permute[j]] // oggenc/audio.c:619

While oggdec has the following code:

	permute = {0,2,1,5,3,4}                 // oggdec/oggdec.c:229
	for (j)
	    to_wav[permute[j]] = from_vorbis[j] // oggdec/oggdec.c:240

You can notice the permutation vectors are reciprocal to each other and that
the code apply them in the opposite direction. Both inversions cancel
each-other.

After more careful checking with the specs, oggenc is right and oggdec is
wrong.

Note that this is for 5.1; 5.0 seems even wronger and 6.1 and 7.1 are not
implemented.

The obvious fix is to copy-paste the matrix from oggenc to oggdec. Patch
attached.

Regards,

-- 
  Nicolas George

_______________________________________________
Vorbis-dev mailing list
[email protected]
http://lists.xiph.org/mailman/listinfo/vorbis-dev
0001-oggdec-import-permute-matrix-from-oggenc.patch (text/x-diff, 1.5 KB) 
From e4e4981af99e13b4676cc4763ab191189cf07467 Mon Sep 17 00:00:00 2001
From: Nicolas George <[email protected]>
Date: Wed, 4 Jul 2012 12:15:01 +0200
Subject: [PATCH] oggdec: import permute matrix from oggenc.

The previous permutations were inverted while
the code assumes they are not.

Also, the matrix from oggenc goes to 8 channels.

Signed-off-by: Nicolas George <[email protected]>
---
 oggdec/oggdec.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/oggdec/oggdec.c b/oggdec/oggdec.c
index 5ffb125..f7ff208 100644
--- a/oggdec/oggdec.c
+++ b/oggdec/oggdec.c
@@ -225,13 +225,21 @@ static FILE *open_output(char *outfile)
 static void
 permute_channels(char *in, char *out, int len, int channels, int bytespersample)
 {
-    int permute[6][6] = {{0}, {0,1}, {0,2,1}, {0,1,2,3}, {0,1,2,3,4}, 
-        {0,2,1,5,3,4}};
+    int permute[6][6] = {
+        {0},              /* 1.0 mono   */
+        {0,1},            /* 2.0 stereo */
+        {0,2,1},          /* 3.0 channel ('wide') stereo */
+        {0,1,2,3},        /* 4.0 discrete quadraphonic */
+        {0,2,1,3,4},      /* 5.0 surround */
+        {0,2,1,4,5,3},    /* 5.1 surround */
+        {0,2,1,5,6,4,3},  /* 6.1 surround */
+        {0,2,1,6,7,4,5,3} /* 7.1 surround (classic theater 8-track) */
+    };
     int i,j,k;
     int samples = len/channels/bytespersample;
 
     /* Can't handle, don't try */
-    if (channels > 6)
+    if (channels > 8)
         return;
 
     for (i=0; i < samples; i++) {
-- 
1.7.10
signature.asc (application/pgp-signature, 198 B) 
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)

iEYEARECAAYFAk/0GGQACgkQsGPZlzblTJMaZACgh7YmwT1jaW2WgpbooQD7XVw7
XOEAnR0t0yTdmRPVdfR49t01IT5UaWHG
=yHjN
-----END PGP SIGNATURE-----