[FFmpeg-devel] Correct libvorbis.c behaviour with >2 channels

[James Darnley]

On 17 June 2010 10:21, Rob <robert.swain at gmail.com> wrote:
> On 3 June 2010 11:35, James Darnley <james.darnley at gmail.com> wrote:
>> On 3 June 2010 00:08, Justin Ruggles <justin.ruggles at gmail.com> wrote:
>>> There are some patches on the issue tracker that might be useful.
>>>
>>> https://roundup.ffmpeg.org/issue1325
>>>
>>> This was only for 5.1 channels though. ?The vorbis spec has been
>>> modified since then to support up to 7.1 I think... or more?
>>
>> Yes, it now has mappings for 1-8 channels.
>>
>> Any comments about putting the following in vorbis_data.c?
>>
>>>const uint8_t ff_vorbis_encoding_channel_layout_offsets[8][8] = {
>>> ? ?{ 0, },
>>> ? ?{ 0, 1, },
>>> ? ?{ 0, 2, 1, },
>>> ? ?{ 0, 1, 2, 3, },
>>> ? ?{ 0, 2, 1, 3, 4, },
>>> ? ?{ 0, 2, 1, 4, 5, 3, },
>>> ? ?{ 0, 2, 1, 5, 6, 4, 3, },
>>> ? ?{ 0, 2, 1, 6, 7, 4, 5, 3},
>>>};
>
> These look correct and vorbis_data.c is the correct location I guess.
> Unless it is only used for decoding, then I'm not sure.
>
>> Also, what should I do if someone tries to use more that 8 channels.
>> Libvorbis appears to support it. ?Should this wrapper support it too
>> or return an error?
>
> No. More channels are valid, but their order is defined by the
> application as I recall. So >8 channels should just be pushed through
> in the order they are in. Or maybe some might prefer that the first 8
> channels of the output match the order for 8 channels and then the
> rest are in the order as the come.
>

Okay...
Table added to vorbis_data.c
Channel order not changed with more than 8 channels

patch attached
-------------- next part --------------
>From 22c47adbf323f2a24ce42aa6376d32f43d8657d9 Mon Sep 17 00:00:00 2001
From: James Darnley <james.darnley at gmail.com>
Date: Fri, 28 May 2010 14:36:53 +0200
Subject: [PATCH 2/6] Fix libvorbis encoding with more than 2 channels

---
 libavcodec/Makefile      |    2 +-
 libavcodec/libvorbis.c   |   13 ++++++-------
 libavcodec/vorbis.h      |    1 +
 libavcodec/vorbis_data.c |   11 +++++++++++
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 8850f19..7840dd2 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -538,7 +538,7 @@ OBJS-$(CONFIG_LIBSCHROEDINGER_ENCODER)    += libschroedingerenc.o \
                                              libdirac_libschro.o
 OBJS-$(CONFIG_LIBSPEEX_DECODER)           += libspeexdec.o
 OBJS-$(CONFIG_LIBTHEORA_ENCODER)          += libtheoraenc.o
-OBJS-$(CONFIG_LIBVORBIS_ENCODER)          += libvorbis.o
+OBJS-$(CONFIG_LIBVORBIS_ENCODER)          += libvorbis.o vorbis_data.o
 OBJS-$(CONFIG_LIBVPX_DECODER)             += libvpxdec.o
 OBJS-$(CONFIG_LIBVPX_ENCODER)             += libvpxenc.o
 OBJS-$(CONFIG_LIBX264_ENCODER)            += libx264.o
diff --git a/libavcodec/libvorbis.c b/libavcodec/libvorbis.c
index a7044a2..52ab60a 100644
--- a/libavcodec/libvorbis.c
+++ b/libavcodec/libvorbis.c
@@ -28,6 +28,7 @@
 
 #include "avcodec.h"
 #include "bytestream.h"
+#include "vorbis.h"
 
 #undef NDEBUG
 #include <assert.h>
@@ -146,16 +147,14 @@ static int oggvorbis_encode_frame(AVCodecContext *avccontext,
     if(data) {
         int samples = OGGVORBIS_FRAME_SIZE;
         float **buffer ;
+        int c, channels = context->vi.channels;
 
         buffer = vorbis_analysis_buffer(&context->vd, samples) ;
-        if(context->vi.channels == 1) {
+        for (c = 0; c < channels; c++) {
+            int co = (channels > 8) ? c :
+                ff_vorbis_encoding_channel_layout_offsets[channels-1][c];
             for(l = 0 ; l < samples ; l++)
-                buffer[0][l]=audio[l]/32768.f;
-        } else {
-            for(l = 0 ; l < samples ; l++){
-                buffer[0][l]=audio[l*2]/32768.f;
-                buffer[1][l]=audio[l*2+1]/32768.f;
-            }
+                buffer[c][l]=audio[l*channels+co]/32768.f;
         }
         vorbis_analysis_wrote(&context->vd, samples) ;
     } else {
diff --git a/libavcodec/vorbis.h b/libavcodec/vorbis.h
index ce9bead..64bade6 100644
--- a/libavcodec/vorbis.h
+++ b/libavcodec/vorbis.h
@@ -26,6 +26,7 @@
 extern const float ff_vorbis_floor1_inverse_db_table[256];
 extern const float * const ff_vorbis_vwin[8];
 extern const uint8_t ff_vorbis_channel_layout_offsets[8][8];
+extern const uint8_t ff_vorbis_encoding_channel_layout_offsets[8][8];
 extern const int64_t ff_vorbis_channel_layouts[9];
 
 typedef struct {
diff --git a/libavcodec/vorbis_data.c b/libavcodec/vorbis_data.c
index 9bc7979..c7dbe7a 100644
--- a/libavcodec/vorbis_data.c
+++ b/libavcodec/vorbis_data.c
@@ -32,6 +32,17 @@ const uint8_t ff_vorbis_channel_layout_offsets[8][8] = {
     { 0, 2, 1, 7, 5, 6, 3, 4},
 };
 
+const uint8_t ff_vorbis_encoding_channel_layout_offsets[8][8] = {
+    { 0, },
+    { 0, 1, },
+    { 0, 2, 1, },
+    { 0, 1, 2, 3, },
+    { 0, 2, 1, 3, 4, },
+    { 0, 2, 1, 4, 5, 3, },
+    { 0, 2, 1, 5, 6, 4, 3, },
+    { 0, 2, 1, 6, 7, 4, 5, 3},
+};
+
 const int64_t ff_vorbis_channel_layouts[9] = {
     CH_LAYOUT_MONO,
     CH_LAYOUT_STEREO,
-- 
1.7.0.4