[FFmpeg-devel] [PATCH] add coded_channels to audio api

[Michael Niedermayer]

On Sun, Jan 06, 2008 at 01:36:48AM -0500, Justin Ruggles wrote:
> Hello,
> 
> This is a huge patch to add a coded_channels variable to AVCodecContext.
>    It's only used on the decoding side.
> 
> The design concept is that avctx.coded_channels should always be the
> actual number of channels in the source stream, avctx.request_channels
> is the number of channels that the user wants the decoder to convert to,
> and avctx.channels is the actual number of channels output by the
> decoder (which may or may not match request_channels).
> 
> So all of the demuxers don't touch avctx.channels, only
> avctx.coded_channels.  The same goes for parsers.  All decoders are
> required to set avctx.channels.  They can either set it to
> avctx.coded_channels or avtx.request_channels.
> 
> It will probably require a good bit of testing since it impacts so many
> decoders and demuxers.
> 
> -Justin

> Index: ffmpeg.c
> ===================================================================
> --- ffmpeg.c	(revision 11431)
> +++ ffmpeg.c	(working copy)
> @@ -2722,8 +2722,8 @@
>                  if(str && (opt->flags & AV_OPT_FLAG_AUDIO_PARAM) && (opt->flags & AV_OPT_FLAG_DECODING_PARAM))
>                      av_set_string(enc, opt_names[j], str);
>              }
> -            //fprintf(stderr, "\nInput Audio channels: %d", enc->channels);
> -            audio_channels = enc->channels;
> +            //fprintf(stderr, "\nInput Audio channels: %d", enc->coded_channels);
> +            audio_channels = enc->coded_channels;
>              audio_sample_rate = enc->sample_rate;
>              if(audio_disable)
>                  ic->streams[i]->discard= AVDISCARD_ALL;

i think this should be in a seperate patch (same for ffplay.c)
and did you test both normal transcoding and stream copy with request!=coded!=channels
?


> Index: libavcodec/nellymoserdec.c
> ===================================================================
> --- libavcodec/nellymoserdec.c	(revision 11431)
> +++ libavcodec/nellymoserdec.c	(working copy)
> @@ -353,6 +353,8 @@
>              sine_window[i] = sin((i + 0.5) / 256.0 * M_PI);
>          }
>  
> +    avctx->channels = avctx->coded_channels;
> +
>      return 0;
>  }

i think that maybe a CODEC_CAP_DOWNMIX would be informative and would
greatly reduce comlexity as codecs which didnt support downmixing channels
could get their coded_channels/channels set by generic code instead of
haveing to set both

also a bigger issue is that this patch as is would require major version
bumps for both libavcodec and libavformat

in the past demuxers set channels, i think channels is 0 now this breaks
ABI and API, and iam against this
actually i doubt most of the changes of this patch are needed or make sense
demuxers can keep setting channels, the value they code has as much chance
to match the decoder output as it has to match the coded number.
generic code in lavf can fill in coded_channels from the channels value of
the demuxer

so please try to not break the whole ABI and API

and split the patch in libavcodec and libavformat patches (if this fails its
a good sign that theres some ABI/API breakage)

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080119/38142b6d/attachment.pgp>