[FFmpeg-devel] patch for DCA "floating point output" option

[Måns Rullgård]

Michael Niedermayer <michaelni at gmx.at> writes:

> On Sat, Jan 19, 2008 at 05:10:01PM +0100, madshi wrote:
>> M?ns Rullg?rd schrieb:
>> > madshi <dear at madshi.net> writes:
>> >
>> >   
>> >> M?ns Rullg?rd schrieb:
>> >>     
>> >>> madshi <dear at madshi.net> writes:
>> >>>
>> >>>   
>> >>>       
>> >>>> M?ns Rullg?rd schrieb:
>> >>>>     
>> >>>>         
>> >>>>> madshi <dear at madshi.net> writes:
>> >>>>>
>> >>>>>   
>> >>>>>       
>> >>>>>           
>> >>>>>> Not sure whether the DCA maintainers will like this patch. But maybe
>> >>>>>> they will, so I'm posting it here just in case.
>> >>>>>>
>> >>>>>> This patch adds optional floating point output to the DCA
>> >>>>>> decoder. This optional feature must be enabled at compile time by
>> >>>>>> adding the following switch to config.h:
>> >>>>>>
>> >>>>>> #define CONFIG_AUDIO_NONSHORT 1
>> >>>>>>
>> >>>>>> I'm using this solution in my eac3to tool because 16bit PCM just
>> >>>>>> doesn't cut it for a good audio transcoding tool. The idea for this
>> >>>>>> option comes from the MLP/TrueHD decoder which offers a similar
>> >>>>>> feature.
>> >>>>>>
>> >>>>>> +#ifdef CONFIG_AUDIO_NONSHORT
>> >>>>>>     
>> >>>>>>         
>> >>>>>>             
>> >>>>> That's a rather useless name.  Can't you think of anything better?
>> >>>>>
>> >>>>>   
>> >>>>>       
>> >>>>>           
>> >>>> It was not my idea. The same constant is already used by the
>> >>>> MLP/TrueHD decoder. I thought that it doesn't make sense to
>> >>>> use a different name for every decoder...
>> >>>>
>> >>>> Besides, I don't think that the name is useless. Currently the decoders
>> >>>> all output "short" samples (shorter than they should be). "non short"
>> >>>> simply means that the decoders stop shorten the audio samples.
>> >>>>     
>> >>>>         
>> >>> The name is equally non-descriptive for any decoder.  If the output is
>> >>> not 'short', then what is it?
>> >>>   
>> >>>       
>> >> Well, the point of the constant is to allow the decoder to output
>> >> whatever sample format suits best. It may be floating point or
>> >> 24 bit integer or even the usual 16 bit integer. Of course the
>> >> decoder must set the "sample_fmt" field to the correct value.
>> >> My patch does that. The MLP/TrueHD decoder does that, too.
>> >>     
>> >
>> > I figured as much, but the name doesn't convey that very well.
>> > Perhaps something like "native" would be better than "nonshort".
>> >   
>> 
>> "native" sounds very good to me!
>
> Changing the name from CONFIG_AUDIO_NONSHORT will break applications using
> it, also it is independant of this patch, that is
> adding float output support vs. changing API.

How can renaming a CONFIG_ symbol break the API?  In fact, I can't see
that's ever set, so renaming it will certainly make no difference.

>> >> Personally, I want every decoder to output the most ideal
>> >> format and bitdepth for the audio track. And this is not the
>> >> same for every decoder. So the "non short" flag just tells the
>> >> decoder to output what it likes best.
>> >>     
>> >
>> > I agree.  This is how video is handled.  To do this for audio, we'd
>> > first need a generic way of converting between different sample
>> > formats.
>> >   
>> 
>> For ffmpeg yes. For libav not necessarily. Personally, I'm only
>
> Yes, adding float support to ffmpeg.c or generic audio format convertion
> to lavc are all seperate things. No need to make them artificially dependant

What I'm saying is that if we change the output format of audio
decoders, we will have to change ffmpeg.c somehow, or it simply will
not work.

-- 
M?ns Rullg?rd
mans at mansr.com