#156 Building for Android, decwin table created too small

svn
closed-fixed
nobody
mpglib (31)
5
2014-06-21
2011-04-17
Dale King
No

Building for Android (OPT_ARM) was experiencing crashes just requesting the format of a file. Turns out the problem is that the decwin table is created too small.

The code for OPT_ARM to compute size looks like this:

    int decwin\_size = \(512+32\)\*sizeof\(real\);

...
#if defined(OPT_ALTIVEC) || defined(OPT_ARM)
if(decwin_size < (512+32)*4) decwin_size = (512+32)*4;
decwin_size += 512*4;

The problem is the number 4 in there. Real is 8 bytes on Android, not 4. So they table comes out as 6400 bytes when it really needs to be 8448 bytes and when it gets initialized it overflows the allocated memory creating random crashes.

Discussion

  • Thomas Orgis
    Thomas Orgis
    2011-04-17

    OK ... I changed the final decwin_size += 512*4; to decwin_size += 512*sizeof(real); Makes sense in any case. Would you test http://mpg123.org/download/mpg123-1.13.3.tar.bz2 to verify that the next release has it fixed?

    Nut: I do wonder ... you really have 64 bit native integers on Android? Aren't ARM registers still 32 bit? Perhaps the code would run better if we used a 32bit type. I repeat: You do havea system with _native_ 64 bit long? Or does the Android software emulate that? In the latter case ... I suppose the performance you get is not very stellar.

    Oh, in any case: You are not on a Cortex A8 cpu where you can use --with-cpu=neon instead? That should be a lot faster. But clearing up my confusion about the 64 bit long would be nice, too.

     
  • Dale King
    Dale King
    2011-04-17

    Sorry, I guess that is my bad. I was not going through the whole auto-conf and auto-make process for building the library. I was just building from the source code in libmpg123 and adding some defines to config.h.

    I didn't define any of the REAL_IS_ macros so it just went to the #else case of making real be double.

    I did an autoconf to figure out what I should put into config.h and now I have a working decoder!!!

    But as you said technically it should be sizeof(real) since that is actually how it will be accessed.

     
  • Thomas Orgis
    Thomas Orgis
    2011-04-21

    • status: open --> closed-fixed