[FFmpeg-devel] [PATCH] Use more appropriate types when decoding vorbisfloats

[Michael Niedermayer]

On Sat, Jul 24, 2010 at 05:19:02AM +0200, Rafa?l Carr? wrote:
> On Sat, 24 Jul 2010 04:23:03 +0200
> Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> > On Sat, Jul 24, 2010 at 04:15:18AM +0200, Rafa?l Carr? wrote:
> > > On Fri, 23 Jul 2010 18:10:45 -0400
> > > Alex Converse <alex.converse at gmail.com> wrote:
> > > 
> > > Hi,
> > > 
> > > > Index: libavcodec/vorbis_dec.c
> > > > ===================================================================
> > > > --- libavcodec/vorbis_dec.c	(revision 24471)
> > > > +++ libavcodec/vorbis_dec.c	(working copy)
> > > > @@ -178,11 +178,11 @@
> > > >  
> > > >  static float vorbisfloat2float(uint_fast32_t val)
> > > >  {
> > > > -    double mant = val & 0x1fffff;
> > > > -    long exp    = (val & 0x7fe00000L) >> 21;
> > > > +    float mant = val & 0x1fffff;
> > > > +    int exp    = (val & 0x7fe00000L) >> 21;
> > >                                      ^
> > > Shouldn't the L be dropped too?
> > 
> > patcheck is sad that it cant detect that, want to make it happy with a
> > patch?
> 
> Does this make patcheck happy?
> 
> diff --git a/tools/patcheck b/tools/patcheck
> index 521138a..62df31e 100755
> --- a/tools/patcheck
> +++ b/tools/patcheck
> @@ -89,6 +89,7 @@ hiegrep ':\+[^}#]*else' 'missing } prior to else' $*
>  #FIXME this should print the previous statement maybe
>  hiegrep ':\+  *{ *$' '{ should be on the same line as the related previous statement' $*
>  
> +hiegrep 'int.*([[:space:]]|=|\*|^|\+|-|/|\||\()([0-9]+|0[xX][a-fA-F0-9]+)L' 'possibly mixing int variable and long constant' $*
>  
>  rm $TMP
>  for i in `grep -H '^+.*@param' $*| sed
>  's/^\([^:]*\):.*@param\(\[.*\]\|\) *\([a-zA-Z0-9_]*\) .*$/\1:\3/'` ; do
> 
> 
> Matching (, space and operators avoid matching defines like FF_QP"2L"AMBDA
> Also I assume 'long' will not be written 'long int' although there's at 
> least 3 occurences already (for example lrint* prototypes)
> 
> 
> 
> Running grep -rIE regexp on ffmpeg/ it shows some int64_t = ... 1LL ...
> Is that ok or should it rather use less-readable (int64_t)constant ?

i was thinkng of a regexp like:
'[^0-9a-zA-Z._][0-9]*[0-9]L[^a-zA-Z0-9]'

or are L nums ever usefull on posix with int>=32bit and long>=32bit?




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

I have often repented speaking, but never of holding my tongue.
-- Xenocrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100725/da4ef07e/attachment.pgp>