Bug in Bink preloading (BinkPluginPreload)

We are trying to seamlessly play a sequence of Bink files. There’s a tiny gap noticeable between them, so I tried to use BinkPluginPreload to preload all the Bink files.

However, there’s a bug in the Bink preloading functions that will cause it to crash on all platforms where sizeof(chartype) != 1.

  • BinkPluginPreload goes into preload_bink, which at the end does this:

predata[ i ] = (BINKPRELOADDATA*)(((char*)lf)-1024); predata[ i ]->offset = file_byte_offset; strcopier( predata[ i ]->name, name ); // BINKFILEDATALOADONLY allocates 1024 bytes in front of the bink header for user data* BINKPRELOADDATA looks like this

typedef struct BINKPRELOADDATA { U64 offset; chartype name[1024-8]; BINK b; } BINKPRELOADDATA;* The expectation is that BINKPRELOADDATA::b aligns with the lf being stored in predata[i], which it does on platforms where chartype is 1 byte. However, on some platforms it’s 2 bytes:

#if defined(__RADWIN__) || defined(__RADNT__) || defined(USE_UTF16) //... #define chartype unsigned short #else #define chartype char #endif* On those platforms, the struct members are misaligned with the 1024 “header” in BINKPRELOADDATA, and then when the Bink file actually gets opened later on, it tries to access the lf stored in the preload data via the BINKPRELOADDATA::b member, which points at garbage:

int p = find_preloaded_name( name, file_byte_offset ); if ( p >= 0 ) { name = (const chartype *)predata[p]->b.preloadptr; flags |= BINKFROMMEMORY; o.FileOffset = 0; }

Fix would be to either not hardcode the 1024 bytes offset and make that depend on the chartype size, or to reduce the amount of storable characters in BINKPRELOADDATA::name depending on chartype size.

We are likely going to go a completely different route to deal with this, which doesn’t rely on preloading, because we don’t feel like setting up custom builds of Bink for all platforms we need, but I’m dropping this here in hopes that it might eventually get fixed and spare someone else the headache.

Ciao, Daniel!

We will check it out!!