Re: [Aften-devel] [PATCH] multiple mono input

[Prakash P. <pr...@pu...>]

On the day of Sunday 11 November 2007 Justin Ruggles hast written:
> Here is an updated patch against SVN-r614.  It is less messy and less
> buggy than the first patch.  I'm going to do some more complete tests

Hi, some comments:

+#define CHANNEL_INTERLEAVE_COMMON(DATA_TYPE) \
+{ \
+ =A0 =A0DATA_TYPE *input =3D (DATA_TYPE *)buf_ptr; \

Is this correct? Shouldn't it be buf instead of buf_ptr:

+    buf_ptr =3D buf;
+    for(i=3D0; i<pfm->channels; i++) {
[...]
+        samples_read =3D MAX(samples_read, nr);
+        buf_ptr +=3D chansize;
+    }
+
+    /* interleave samples to create multichannel */
+    switch(pfm->read_format) {
+        case PCM_SAMPLE_FMT_U8:
+            CHANNEL_INTERLEAVE_COMMON(uint8_t)


+int
+pcmfile_read_samples_multi(PcmFileMulti *pfm, void *buffer, int num_sample=
s)
+{
+ =A0 =A0int i;
+ =A0 =A0int samples_read, chansize;
+ =A0 =A0uint8_t *buf;
+ =A0 =A0uint8_t *buf_ptr;
+
+ =A0 =A0/* allocate buffer */
+ =A0 =A0chansize =3D num_samples * sample_sizes[pfm->read_format];
+ =A0 =A0buf =3D calloc(1, chansize * pfm->channels);

Is malloc sufficient? Or is this for the case of eof? Then you should use=20
memset if nr < expected.

=46urthermore it might be good to check filesize, if they match - or you co=
uld=20
also allow different filesizes, but then the logic would be a bit more=20
complex.

+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0} else if(argv[i][1] =3D=3D 'o') {
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0i++;
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if(i >=3D argc) return 1;
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0opts->outfile =3D argv[i];
+ =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0found_output =3D 1;
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}

I am not sure that I like this, esp as it changes aften usage. Instead of

aften FL.wav FR.wav C.wav LFE.wav SL.wav SR.wav -o t_32+LFE.ac3

perhepas something like

aften -ch_sources (FL.wav FR.wav C.wav LFE.wav SL.wav SR.wav) t_32+LFE.ac3

Perhaps for the user it might even be simpler if each channel is specified=
=20
with a diffent param:

aften -ch_fl FL.wav -ch_fr FR.wav t.ac3

Then=20
a) The order in which input files are specified doesn't matter.
b) The user doesn't have to sync the order to a separate acmod/channelconfi=
g=20
param.


+ =A0 =A0FILE *ifp[6];

Should be inited with NULLs.

+ =A0 =A0 =A0 =A0ch_mask =3D 0x80000000;

No magic numbers please.

+        if(opts.num_input_files > 1)
+            nr =3D pcmfile_read_samples_multi(&pfm, fwav, 256);
+        else
+            nr =3D pcmfile_read_samples(&pf, fwav, 256);

I wonder whether the signature of both functions should be made the same, s=
o=20
that you could work with a function pointer here.


Another thing: I think we should add the possibility to call the encode=20
function with non-interleaved buffers. Esp with mono input you save yoursel=
f=20
the interleaving and deinterleaving in the encoder. I also think this makes=
=20
the aften library more flexible to use, and save some cycles if your sample=
s=20
are available non-interleaved.

Cheers,
=2D-=20
(=B0=3D                 =3D=B0)
//\ Prakash Punnoor /\\
V_/                 \_V