[FFmpeg-devel] [PATCH] floating-point input to libvorbis encoder

[Justin Ruggles]

Michael Niedermayer wrote:

> On Mon, Sep 20, 2010 at 08:32:46AM +0100, M?ns Rullg?rd wrote:
>> Tomas H?rdin <tomas.hardin at codemill.se> writes:
>>
>>> On Sat, 2010-09-18 at 12:12 -0400, Justin Ruggles wrote:
>>>> Hi,
>>>>
>>>> libvorbis takes floating-point samples as input.  Our libvorbis wrapper
>>>> currently only accepts 16-bit input and converts it to float.  It seems
>>>> better to me to have it only accept float input and leave the conversion
>>>> outside of the encoder.
>>>>
>>>> -Justin
>>>>
>>>> plain text document attachment (libvorbis_float_input.patch)
>>>> Index: libavcodec/libvorbis.c
>>>> ===================================================================
>>>> --- libavcodec/libvorbis.c	(revision 25138)
>>>> +++ libavcodec/libvorbis.c	(working copy)
>>>> @@ -145,7 +145,7 @@
>>>>  {
>>>>      OggVorbisContext *context = avccontext->priv_data ;
>>>>      ogg_packet op ;
>>>> -    signed short *audio = data ;
>>>> +    float *audio = data;
>>>>      int l;
>>>>  
>>>>      if(data) {
>>>> @@ -158,7 +158,7 @@
>>>>              int co = (channels > 8) ? c :
>>>>                  ff_vorbis_encoding_channel_layout_offsets[channels-1][c];
>>>>              for(l = 0 ; l < samples ; l++)
>>>> -                buffer[c][l]=audio[l*channels+co]/32768.f;
>>>> +                buffer[c][l] = audio[l*channels+co];
>>>>          }
>>>>          vorbis_analysis_wrote(&context->vd, samples) ;
>>>>      } else {
>>>> @@ -238,6 +238,6 @@
>>>>      oggvorbis_encode_frame,
>>>>      oggvorbis_encode_close,
>>>>      .capabilities= CODEC_CAP_DELAY,
>>>> -    .sample_fmts = (const enum SampleFormat[]){SAMPLE_FMT_S16,SAMPLE_FMT_NONE},
>>>> +    .sample_fmts = (const enum SampleFormat[]){SAMPLE_FMT_FLT,SAMPLE_FMT_NONE},
>>>>      .long_name= NULL_IF_CONFIG_SMALL("libvorbis Vorbis"),
>>>>  } ;
>>> Please correct me if I'm wrong, but doesn't this require a major version
>>> bump?
>> Strictly speaking, no.  The codec exports the list of supported input
>> formats, and a proper app should perform any conversions necessary.
>> That said, there are probably many apps assuming s16 is supported, and
>> they would indeed be broken by this change.  Do we care about those?
> 
> in this case i dont think we care, apps should implement the api properly
> and its quite trivial to fix an app that does not

So is the patch ok?

-Justin