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

[Michael Niedermayer]

On Sat, Sep 25, 2010 at 01:12:55PM -0400, Justin Ruggles wrote:
> 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?

as far as iam concerned yes if its tested

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

It is not what we do, but why we do it that matters.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100926/3c4a0181/attachment.pgp>