[FFmpeg-devel] Correct libvorbis.c behaviour with >2 channels
James Darnley
james.darnley
Sun Jun 27 11:17:03 CEST 2010
On 27 June 2010 06:38, David Conrad <lessen42 at gmail.com> wrote:
> On Jun 23, 2010, at 4:35 PM, James Darnley wrote:
>
>> 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
>
>
>> +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},
>> +};
>> +
>
> Leave out the trailing comma and add a space before the } for 8 channels.
>
> Patch OK with that fixed
Done. Have a free cosmetics patch too.
-------------- next part --------------
>From 5e230fcb31e8ea15cf4f1b6e7e353d42fa414bc3 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/7] 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 e5b40a4..c0531c6 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -540,7 +540,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 5926d98..f6872b2 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..c504664 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
-------------- next part --------------
>From a1b6fed19a1fd267727164ef680cdfa129b930bc Mon Sep 17 00:00:00 2001
From: James Darnley <james.darnley at gmail.com>
Date: Sun, 27 Jun 2010 11:09:40 +0200
Subject: [PATCH 7/7] Cosmetics
---
libavcodec/vorbis_data.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/libavcodec/vorbis_data.c b/libavcodec/vorbis_data.c
index c504664..0674ac5 100644
--- a/libavcodec/vorbis_data.c
+++ b/libavcodec/vorbis_data.c
@@ -29,7 +29,7 @@ const uint8_t ff_vorbis_channel_layout_offsets[8][8] = {
{ 0, 2, 1, 3, 4, },
{ 0, 2, 1, 5, 3, 4, },
{ 0, 2, 1, 6, 5, 3, 4, },
- { 0, 2, 1, 7, 5, 6, 3, 4},
+ { 0, 2, 1, 7, 5, 6, 3, 4 }
};
const uint8_t ff_vorbis_encoding_channel_layout_offsets[8][8] = {
--
1.7.0.4
>From Diego Sun Jun 27 11:22:35 2010
From: Diego (Diego)
Date: Sun, 27 Jun 2010 11:22:35 +0200
Subject: [FFmpeg-devel] [PATCH 2/2] tablegen: implement and use
tablegen_copy_array macros
In-Reply-To: <20100627070350.GA2095 at 1und1.de>
References: <1277599253-29113-1-git-send-email-flameeyes at gmail.com>
<1277599253-29113-2-git-send-email-flameeyes at gmail.com>
<20100627070350.GA2095 at 1und1.de>
Message-ID: <1277630555.11106.55.camel at yamato>
Il giorno dom, 27/06/2010 alle 09.03 +0200, Reimar D?ffinger ha scritto:
> Also, since it is a macro, I think uppercase is more approriate.
> Why not
> WRITE_ARRAY
> and
> WRITE_2D_ARRAY?
Mostly I was trying to differentiate between the two definition macros
we've got already and these that are "action macros", but I have no
particular preference...
> Michael doesn't like it, but I also slightly prefer macros
> to be encased in "do {} while (0)", which would also allow
> you to use a variable for FF_ARRAY_ELEMS(name) instead
> of writing it multiple times.
All fine by me, I'll rework with that.
> One thing I am undecided about is if it would be nicer to use
> a string for "modifier" and/or call it "prefix".
That's a bit of a doubt for me as well... guess having a string might
actually make sense for eventual attributes or whatever else.
v2 will come either before lunch or tomorrow ^^;;
--
Diego Elio Petten? ? ?Flameeyes?
http://blog.flameeyes.eu/
If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/
>From Diego Sun Jun 27 11:27:38 2010
From: Diego (Diego)
Date: Sun, 27 Jun 2010 11:27:38 +0200
Subject: [FFmpeg-devel] [PATCH] Change broken -Werror=implicit flag in
configure into working flag
In-Reply-To: <AANLkTikH_blnI33UAet8soon9HlhSzSyjsibb5is7DAh at mail.gmail.com>
References: <AANLkTikH_blnI33UAet8soon9HlhSzSyjsibb5is7DAh at mail.gmail.com>
Message-ID: <1277630858.11106.56.camel at yamato>
Il giorno sab, 26/06/2010 alle 19.21 -0700, Eli Friedman ha scritto:
>
> Patch attached. At least with gcc 4.4.3, -Werror=implicit does
> nothing, and -Werror=implicit-int does not work with -std=c99.
Good catch; fwiw this is fixed for either 4.5 or 4.6, actually can't
tell:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40989
--
Diego Elio Petten? ? ?Flameeyes?
http://blog.flameeyes.eu/
If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/
More information about the ffmpeg-devel
mailing list