[FFmpeg-cvslog] r18623 - trunk/libavcodec/ac3enc.c

[Justin Ruggles]

Michael Niedermayer wrote:
> On Sun, Apr 19, 2009 at 05:06:13PM +0200, jbr wrote:
>> Author: jbr
>> Date: Sun Apr 19 17:06:13 2009
>> New Revision: 18623
>>
>> Log:
>> Add channel layout support to the AC-3 encoder.
>>
>> Modified:
>>    trunk/libavcodec/ac3enc.c
>>
>> Modified: trunk/libavcodec/ac3enc.c
>> ==============================================================================
>> --- trunk/libavcodec/ac3enc.c	Sun Apr 19 17:05:32 2009	(r18622)
>> +++ trunk/libavcodec/ac3enc.c	Sun Apr 19 17:06:13 2009	(r18623)
>> @@ -30,6 +30,7 @@
>>  #include "get_bits.h" // for ff_reverse
>>  #include "put_bits.h"
>>  #include "ac3.h"
>> +#include "audioconvert.h"
>>  
>>  typedef struct AC3EncodeContext {
>>      PutBitContext pb;
>> @@ -609,37 +610,67 @@ static int compute_bit_allocation(AC3Enc
>>      return 0;
>>  }
>>  
>> +static av_cold int set_channel_info(AC3EncodeContext *s, int channels,
>> +                                    int64_t *channel_layout)
>> +{
>> +    int ch_layout;
>> +
>> +    if (channels < 1 || channels > AC3_MAX_CHANNELS)
>> +        return -1;
>> +    if ((uint64_t)*channel_layout > 0x7FF)
>> +        return -1;
>> +    ch_layout = *channel_layout;
> 
>> +    if (!ch_layout)
>> +        ch_layout = avcodec_guess_channel_layout(channels, CODEC_ID_AC3, NULL);
> 
> this is incorrect the way it is used
> the channel layout must be set on the demuxer/decoder side, one cannot
> guess the layout from the decoder or the user app by using the encoder
> CODEC_ID.

The way it is done currently in ffmpeg.c, the encoder doesn't know the
values from the decoder (not initialized yet), only the demuxer and the
commandline.  The demuxer reports the values in the original stream, not
taking into account any change by the decoder for downmixing based on
request_channels.  The recent patch I applied to reset channel_layout to
0 if it didn't match number of channels was done so that the encoder
would at least not see the wrong value based on the demuxer and could
try to guess the best layout based on the number of channels given in
the commandline.  This is not ideal, but better than having the wrong value.

A better solution might be to swap the order of initialization so that
decoders are initialized first, then channels and channel_layout are
copied from the decoder to the encoder before it is initialized.  But
I'm not sure if there is some specific reason that the encoders are
currently initialized first.

-Justin