[FFmpeg-devel] [PATCH] Fix segmentation fault when encoding CODEC_ID_PCM_F32BE frames

[Stefano Sabatini]

On date Sunday 2008-08-10 11:10:30 +1000, pross at xvid.org encoded:
> On Sat, Aug 09, 2008 at 09:54:24PM +0200, Stefano Sabatini wrote:
> > On date Saturday 2008-08-09 21:22:17 +0200, Michael Niedermayer encoded:
> > > On Sat, Aug 09, 2008 at 08:03:55PM +0200, Stefano Sabatini wrote:
> > > > On date Saturday 2008-08-09 18:39:40 +0200, Michael Niedermayer encoded:
> > > > > On Sat, Aug 09, 2008 at 12:40:47PM +0200, Stefano Sabatini wrote:
> > > > > > Hi all,
> > > > > > 
> > > > > > this fixes segfaults in the PCM regression tests.
> > > > > > 
> > > > > > This seems due to the compiler (gcc 4.2) getting confused by this snippet:
> > > > > > 
> > > > > >     {
> > > > > >         int32_t *samples = samples;
> > > > > >         ^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > > > >         for(;n>0;n--) {
> > > > > >             register int32_t v = *samples++ + 0;
> > > > > >             bytestream_put_be32(&dst, v);
> > > > > >         }
> > > > > >     }
> > > > > > 
> > > > > > I'm not sure this is the right fix though.
> > > > > > 
> > > > > > Regards.
> > > > > > -- 
> > > > > > FFmpeg = Foolish and Fast Mastering Proud Excellent Gangster
> > > > > 
> > > > > > Index: libavcodec/pcm.c
> > > > > > ===================================================================
> > > > > > --- libavcodec/pcm.c	(revision 14673)
> > > > > > +++ libavcodec/pcm.c	(working copy)
> > > > > > @@ -171,9 +171,9 @@
> > > > > >   */
> > > > > >  #define ENCODE(type, endian, src, dst, n, offset) \
> > > > > >  { \
> > > > > > -    type *samples = src; \
> > > > > > +    type *samples2 = (type *)src; \
> > > > > 
> > > > > why the cast?
> > > > 
> > > > Fix a warning (yes, this is another patch).
> > > > 
> > > > > and samples2 is hardly the proper solution
> > > > > ENCODE_s would be less likely to clash
> > > > 
> > > > Not understanding here, do you mean samples_ rather than samples2?
> > > 
> > > i mean that using the name of the macro as prefix is likely more
> > > effective than a 2 at the in avoiding a name clash, the _ might
> > > work as well if you prefer
> > 
> > I have no particular preference, and yes I agree suffixing with 2 is
> > not a good idea, if I have to choose I would prefer something which is
> > more consistent with the codebase, so what do you (or others) suggest?
> 
> 
> The next patch in the pcm upgrade(/downgrade) series relocates the working
> 'samples' var into the main function scope. This incidently fixes the problem
> reported here.  Stefano, can you test this before I commit:
> 
> http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/2008-August/050994.html
> 
> Oddly enough the regression segfault did not occur on my test computers
> (gcc-4.1.2/ppc, gcc-4.2.3/x86_64).

Yes, it works just fine here (gcc-4.2.3/i686_32), thanks.

Regards.
-- 
FFmpeg = Fast and Faboulous Multipurpose Proud Egregious Gymnast