[MPlayer-dev-eng] [PATCH] [TEST FUNC] Multi-channel reorder function

Corey Hickey bugfood-ml at fatooh.org
Sat Nov 17 04:55:29 CET 2007


Ulion wrote:
> This patch is just to show the idea, could be buggy and ugly, maybe
> disliked by somebodies, it's ok, I just want to fix the aac wrong
> channel order bug, wellcome any idea to resolve the problem. Shall we
> end this bug in the near future by either way?

I hardly care who's patch gets applied, as long as one of them does...

Here's a small review, to the extent I am able.

> Index: libmpcodecs/ae_faac.c
> ===================================================================
> --- libmpcodecs/ae_faac.c       (revision 25008)
> +++ libmpcodecs/ae_faac.c       (working copy)
> @@ -98,6 +98,15 @@

Missing #include "reorder_copy.h"

> Index: libmpcodecs/ae_pcm.c
> ===================================================================
> --- libmpcodecs/ae_pcm.c	(revision 25008)
> +++ libmpcodecs/ae_pcm.c	(working copy)
> @@ -12,6 +12,7 @@
>  #include "stream/stream.h"
>  #include "libmpdemux/muxer.h"
>  #include "ae_pcm.h"
> +#include "reorder_copy.h"
>  
>  
>  static int bind_pcm(audio_encoder_t *encoder, muxer_stream_t *mux_a)
> @@ -38,6 +39,18 @@
>  static int encode_pcm(audio_encoder_t *encoder, uint8_t *dest, void *src, int nsamples, int max_size)
>  {
>  	max_size = FFMIN(nsamples, max_size);
> +	if (encoder->params.channels == 6 || encoder->params.channels == 5) {
> +		max_size -= max_size % (encoder->params.channels * 2);
> +		if (encoder->params.channels == 6)
> +			reorder_channel_copy(src, AF_CHANNEL_LAYOUT_MPLAYER_5CH_DEFAULT,
> +								 dest, AF_CHANNEL_LAYOUT_WAVEEX_5CH_DEFAULT,
> +								 max_size / 2, 2);

Shouldn't these be AF_CHANNEL_LAYOUT_MPLAYER_6CH_DEFAULT and
AF_CHANNEL_LAYOUT_WAVEEX_6CH_DEFAULT?

> Index: reorder_copy.c
> ===================================================================
> --- reorder_copy.c	(revision 0)
> +++ reorder_copy.c	(revision 0)
> @@ -0,0 +1,872 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <inttypes.h>
> +#include <string.h>
> +#include "libvo/fastmemcpy.h"
> +
> +#include "reorder_copy.h"
> +
> +#define REORDER_COPY_5(DEST,SRC,SAMPLES,S0,S1,S2,S3,S4) \

It seems like all these reorder macros would be nicer as (possibly
inline) functions. Do you have a particular reason for having them as
macros?

> +        printf("[reorder_copy] Unsupported number of bytes/sample: %d, please "\
> +               "report this error on the MPlayer mailing list.\n",SAMPLESIZE);\

This and other printf() calls ought to use mp_msg() instead.

> Index: reorder_copy.h
> ===================================================================
> --- reorder_copy.h	(revision 0)
> +++ reorder_copy.h	(revision 0)
> @@ -0,0 +1,167 @@
> +#ifndef REORDER_COPY_H
> +#define REORDER_COPY_H
> +
> +
> +#define AF_CHANNEL_LABLE_LEFT            1
> +#define AF_CHANNEL_LABLE_RIGHT           2
> +#define AF_CHANNEL_LABLE_CENTER          3
> +#define AF_CHANNEL_LABLE_LFE             4
> +#define AF_CHANNEL_LABLE_LEFT_SURROUND   5
> +#define AF_CHANNEL_LABLE_RIGHT_SURROUND  6
> +#define AF_CHANNEL_LABLE_CENTER_SURROUND 7
> +#define AF_CHANNEL_LABLE_LEFT_CENTER     8
> +#define AF_CHANNEL_LABLE_RIGHT_CENTER    9
> +#define AF_CHANNEL_LABLE_SIDE_LEFT       10
> +#define AF_CHANNEL_LABLE_SIDE_RIGHT      11
> +
> +
> +#define AF_CHANNEL_BIT_LEFT            (1<<0)
> +#define AF_CHANNEL_BIT_RIGHT           (1<<1)
> +#define AF_CHANNEL_BIT_CENTER          (1<<2)
> +#define AF_CHANNEL_BIT_LFE             (1<<3)
> +#define AF_CHANNEL_BIT_LEFT_SURROUND   (1<<4)
> +#define AF_CHANNEL_BIT_RIGHT_SURROUND  (1<<5)
> +#define AF_CHANNEL_BIT_CENTER_SURROUND (1<<6)
> +#define AF_CHANNEL_BIT_LEFT_CENTER     (1<<7)
> +#define AF_CHANNEL_BIT_RIGHT_CENTER    (1<<8)
> +#define AF_CHANNEL_BIT_SIDE_LEFT       (1<<9)
> +#define AF_CHANNEL_BIT_SIDE_RIGHT      (1<<10)
> +
> +// Some channel abbreviations used below:
> +// L - left
> +// R - right
> +// C - center
> +// Ls - left surround
> +// Rs - right surround
> +// Cs - center surround
> +// Rls - rear left surround
> +// Rrs - rear right surround
> +// Lw - left wide
> +// Rw - right wide
> +// Lsd - left surround direct
> +// Rsd - right surround direct
> +// Lc - left center
> +// Rc - right center
> +// Ts - top surround
> +// Vhl - vertical height left
> +// Vhc - vertical height center
> +// Vhr - vertical height right
> +// Lt - left matrix total. for matrix encoded stereo.
> +// Rt - right matrix total. for matrix encoded stereo.
> +
> +#define AF_LFE   (1<<7)
> +
> +#define AF_CHANNEL_LAYOUT_MONO   ((100<<8)|1) // a standard mono stream
> +#define AF_CHANNEL_LAYOUT_STEREO ((101<<8)|2) // a standard stereo stream (L R)
> +
> +//  MPEG defined layouts
> +#define AF_CHANNEL_LAYOUT_MPEG_1_0 AF_CHANNEL_LAYOUT_MONO    // C
> +#define AF_CHANNEL_LAYOUT_MPEG_2_0 AF_CHANNEL_LAYOUT_STEREO  // L R
> +#define AF_CHANNEL_LAYOUT_MPEG_3_0_A ((113<<8)|3)           // L R C
> +#define AF_CHANNEL_LAYOUT_MPEG_3_0_B ((114<<8)|3)           // C L R
> +#define AF_CHANNEL_LAYOUT_MPEG_4_0_A ((115<<8)|4)           // L R C Cs
> +#define AF_CHANNEL_LAYOUT_MPEG_4_0_B ((116<<8)|4)           // C L R Cs
> +#define AF_CHANNEL_LAYOUT_MPEG_5_0_A ((117<<8)|5)           // L R C Ls Rs
> +#define AF_CHANNEL_LAYOUT_MPEG_5_0_B ((118<<8)|5)           // L R Ls Rs C
> +#define AF_CHANNEL_LAYOUT_MPEG_5_0_C ((119<<8)|5)           // L C R Ls Rs
> +#define AF_CHANNEL_LAYOUT_MPEG_5_0_D ((120<<8)|5)           // C L R Ls Rs
> +#define AF_CHANNEL_LAYOUT_MPEG_5_1_A ((121<<8)|6|AF_LFE)    // L R C LFE Ls Rs
> +#define AF_CHANNEL_LAYOUT_MPEG_5_1_B ((122<<8)|6|AF_LFE)    // L R Ls Rs C LFE
> +#define AF_CHANNEL_LAYOUT_MPEG_5_1_C ((123<<8)|6|AF_LFE)    // L C R Ls Rs LFE
> +#define AF_CHANNEL_LAYOUT_MPEG_5_1_D ((124<<8)|6|AF_LFE)    // C L R Ls Rs LFE
> +#define AF_CHANNEL_LAYOUT_MPEG_6_1_A ((125<<8)|7|AF_LFE) // L R C LFE Ls Rs Cs
> +#define AF_CHANNEL_LAYOUT_MPEG_7_1_A ((126<<8)|8|AF_LFE) // L R C LFE Ls Rs Lc Rc
> +#define AF_CHANNEL_LAYOUT_MPEG_7_1_B ((127<<8)|8)  // C Lc Rc L R Ls Rs Rls Rrs
> +#define AF_CHANNEL_LAYOUT_MPEG_7_1_C ((128<<8)|8|AF_LFE) // L R C LFE Ls Rs Rls Rrs
> +#define AF_CHANNEL_LAYOUT_SMPTE_DTV  ((130<<8)|8|AF_LFE) // L R C LFE Ls Rs Lt Rt
> +
> +//  ITU defined layouts
> +#define AF_CHANNEL_LAYOUT_ITU_1_0 AF_CHANNEL_LAYOUT_MONO        // C
> +#define AF_CHANNEL_LAYOUT_ITU_2_0 AF_CHANNEL_LAYOUT_STEREO      // L R
> +#define AF_CHANNEL_LAYOUT_ITU_2_1 ((131<<8)|3)                 // L R Cs
> +#define AF_CHANNEL_LAYOUT_ITU_2_2 ((132<<8)|4)                 // L R Ls Rs
> +#define AF_CHANNEL_LAYOUT_ITU_3_0 AF_CHANNEL_LAYOUT_MPEG_3_0_A  // L R C
> +#define AF_CHANNEL_LAYOUT_ITU_3_1 AF_CHANNEL_LAYOUT_MPEG_4_0_A  // L R C Cs
> +#define AF_CHANNEL_LAYOUT_ITU_3_2 AF_CHANNEL_LAYOUT_MPEG_5_0_A  // L R C Ls Rs
> +#define AF_CHANNEL_LAYOUT_ITU_3_2_1 AF_CHANNEL_LAYOUT_MPEG_5_1_A
> +#define AF_CHANNEL_LAYOUT_ITU_3_4_1 AF_CHANNEL_LAYOUT_MPEG_7_1_C
> +
> +// DVD defined layouts
> +#define AF_CHANNEL_LAYOUT_DVD_0 AF_CHANNEL_LAYOUT_MONO        // C
> +#define AF_CHANNEL_LAYOUT_DVD_1 AF_CHANNEL_LAYOUT_STEREO      // L R
> +#define AF_CHANNEL_LAYOUT_DVD_2 AF_CHANNEL_LAYOUT_ITU_2_1     // L R Cs
> +#define AF_CHANNEL_LAYOUT_DVD_3 AF_CHANNEL_LAYOUT_ITU_2_2     // L R Ls Rs
> +#define AF_CHANNEL_LAYOUT_DVD_4 ((133<<8)|3|AF_LFE)          // L R LFE
> +#define AF_CHANNEL_LAYOUT_DVD_5 ((134<<8)|4|AF_LFE)          // L R LFE Cs
> +#define AF_CHANNEL_LAYOUT_DVD_6 ((135<<8)|5|AF_LFE)          // L R LFE Ls Rs
> +#define AF_CHANNEL_LAYOUT_DVD_7 AF_CHANNEL_LAYOUT_MPEG_3_0_A  // L R C
> +#define AF_CHANNEL_LAYOUT_DVD_8 AF_CHANNEL_LAYOUT_MPEG_4_0_A  // L R C Cs
> +#define AF_CHANNEL_LAYOUT_DVD_9 AF_CHANNEL_LAYOUT_MPEG_5_0_A  // L R C Ls Rs
> +#define AF_CHANNEL_LAYOUT_DVD_10 ((136<<8)|4|AF_LFE)         // L R C LFE
> +#define AF_CHANNEL_LAYOUT_DVD_11 ((137<<8)|5|AF_LFE)         // L R C LFE Cs
> +#define AF_CHANNEL_LAYOUT_DVD_12 AF_CHANNEL_LAYOUT_MPEG_5_1_A // L R C LFE Ls Rs
> +// 13 through 17 are duplicates of 8 through 12.
> +#define AF_CHANNEL_LAYOUT_DVD_13 AF_CHANNEL_LAYOUT_DVD_8
> +#define AF_CHANNEL_LAYOUT_DVD_14 AF_CHANNEL_LAYOUT_DVD_9
> +#define AF_CHANNEL_LAYOUT_DVD_15 AF_CHANNEL_LAYOUT_DVD_10
> +#define AF_CHANNEL_LAYOUT_DVD_16 AF_CHANNEL_LAYOUT_DVD_11
> +#define AF_CHANNEL_LAYOUT_DVD_17 AF_CHANNEL_LAYOUT_DVD_12
> +#define AF_CHANNEL_LAYOUT_DVD_18 ((138<<8)|5|AF_LFE)         // L R Ls Rs LFE
> +#define AF_CHANNEL_LAYOUT_DVD_19 AF_CHANNEL_LAYOUT_MPEG_5_0_B // L R Ls Rs C
> +#define AF_CHANNEL_LAYOUT_DVD_20 AF_CHANNEL_LAYOUT_MPEG_5_1_B // L R Ls Rs C LFE
> +
> +#define AF_CHANNEL_LAYOUT_AAC_3_0 AF_CHANNEL_LAYOUT_MPEG_3_0_B // C L R
> +#define AF_CHANNEL_LAYOUT_AAC_QUAD AF_CHANNEL_LAYOUT_ITU_2_2   // L R Ls Rs
> +#define AF_CHANNEL_LAYOUT_AAC_4_0 AF_CHANNEL_LAYOUT_MPEG_4_0_B // C L R Cs
> +#define AF_CHANNEL_LAYOUT_AAC_5_0 AF_CHANNEL_LAYOUT_MPEG_5_0_D // C L R Ls Rs
> +#define AF_CHANNEL_LAYOUT_AAC_5_1 AF_CHANNEL_LAYOUT_MPEG_5_1_D // C L R Ls Rs LFE
> +#define AF_CHANNEL_LAYOUT_AAC_6_0 ((141<<8)|6)        // C L R Ls Rs Cs
> +#define AF_CHANNEL_LAYOUT_AAC_6_1 ((142<<8)|7|AF_LFE) // C L R Ls Rs Cs LFE
> +#define AF_CHANNEL_LAYOUT_AAC_7_0 ((143<<8)|7)        // C L R Ls Rs Rls Rrs
> +#define AF_CHANNEL_LAYOUT_AAC_7_1 AF_CHANNEL_LAYOUT_MPEG_7_1_B // C Lc Rc L R Ls Rs LFE

I knew this looked familiar...
http://developer.apple.com/documentation/MusicAudio/Reference/CACoreAudioReference/CoreAudioTypes/CompositePage.html

I don't know the license of the above document. Is it OK to include a
derivative work in mplayer?


You're missing a reorder for ao_pcm.c.


I got mplayer to compile with this patch (after adding the missing
#include), but it doesn't seem to be properly reordering the channels on
playback of a 6-channel WAVE file.

http://www-mmsp.ece.mcgill.ca/Documents/AudioFormats/WAVE/Samples/Microsoft/6_Channel_ID.wav

6-channel AAC seems ok.

-Corey



More information about the MPlayer-dev-eng mailing list