NativeInstruments / ni-media

NI Media is a C++ library for reading and writing audio streams.
MIT License
235 stars 34 forks source link

MP3 File Reading may Freeze #73

Closed wro-ableton closed 4 months ago

wro-ableton commented 4 months ago

We seem to have found an issue trying to load the following file. (Zipped so it can be attached) TestMp3.zip

From what I can tell, there seems to be a bug in the code responsible for reading the MP3 file in media_foundation_helper.h L89.

        try
        {
            *read = static_cast<ULONG>( m_source.read( reinterpret_cast<char*>( buffer ), toRead ) );
            return S_OK;
        }

While we're working with template arguments here, m_source in practice is implemented using boost::iostreams::file_descriptor. On windows, this class implements read like this:

    DWORD result;
    if (!::ReadFile(handle_, s, static_cast<DWORD>(n), &result, NULL))
    {
        // report EOF if the write-side of a pipe has been closed
        if (GetLastError() == ERROR_BROKEN_PIPE)
        {
            result = 0;
        }
        else
            throw_system_failure("failed reading");
    }
    return result == 0 ? -1 : static_cast<std::streamsize>(result);

The issue comes from returning -1 in case no bytes were read. SyncronousByteStream::Read casts this to unsigned long and returns 4294967295.

The calling code sadly I can't debug but my assumption would be that it calls Read() until no more bytes are read. Since this cast would cause the number of bytes read to never reach 0, it will continue to read indefinitely or we reach some undefined behaviour.

One solution would be re-interprete -1 (EOF) to 0 (no bytes read), which, from what I can tell, fixes the endless loop and allows ni-media to successfully parse the file.

Why this issue doesn't affect other (most?) mp3 files I don't know. Sadly the documentation on the calling (Windows Media Foundation) code is somewhat limited. :)

Cheers!

ni-mheppner commented 4 months ago

Solved with merging PR #74