[PATCH 0/1] MR6696: mciseq: Don't seek to the end of the root chunk in RMID files.
Commit 9e1008f13735 added mmioSeek call that skips the whole data portion of the RMID chunk, breaking the handling of RMID (RIFF-based MIDI) files as it effectively skips the whole content of the RIFF file. Since mmioDescend expects the file position to be somewhere within the specified parent chunk, it's guaranteed to fail. $ WINEDEBUG=+mcimidi wine wintest.exe mcishell mci.c:178: Type your commands to the MCI, end with Ctrl-Z/^D open BOSSNOVA.RMI mci.c:188: command: open BOSSNOVA.RMI 0764:trace:mcimidi:MIDI_mciOpen (1, 00000200, 0051FB98) 0764:trace:mcimidi:MIDI_mciOpen wDevID=1 (lpParams->wDeviceID=1) 0764:trace:mcimidi:MIDI_mciOpen MCI_OPEN_ELEMENT L"bossnova.rmi"! 0764:trace:mcimidi:MIDI_mciOpen hFile=00000001 0764:trace:mcimidi:MIDI_mciOpen ParentChunk ckid=RIFF fccType=RMID cksize=000052DA mci.c:191: Test failed: mci open BOSSNOVA.RMI error: 296(40 MCIERR_INVALID_FILE) Its introduction seems unrelated to the rest of that commit and there's no reason given for placing it there, so let's remove the mmioSeek call to fix RMID file handling in MCI. Previous mmioDescend call will set the position to just after the chunk's form type, which is exactly where it should be when calling mmioDescend the second time to find the data chunk. Other parts of Wine code that deal with RIFF files don't seek in analogous situations either. Fixes: 9e1008f13735 ("Removed fixed size array to store specific data") -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6696
From: Sebastian Krzyszkowiak <dos(a)dosowisko.net> Commit 9e1008f13735 added mmioSeek call that skips the whole data portion of the RMID chunk, breaking the handling of RMID (RIFF-based MIDI) files as it effectively skips the whole content of the RIFF file. Since mmioDescend expects the file position to be somewhere within the specified parent chunk, it's guaranteed to fail. $ WINEDEBUG=+mcimidi wine wintest.exe mcishell mci.c:178: Type your commands to the MCI, end with Ctrl-Z/^D open BOSSNOVA.RMI mci.c:188: command: open BOSSNOVA.RMI 0764:trace:mcimidi:MIDI_mciOpen (1, 00000200, 0051FB98) 0764:trace:mcimidi:MIDI_mciOpen wDevID=1 (lpParams->wDeviceID=1) 0764:trace:mcimidi:MIDI_mciOpen MCI_OPEN_ELEMENT L"bossnova.rmi"! 0764:trace:mcimidi:MIDI_mciOpen hFile=00000001 0764:trace:mcimidi:MIDI_mciOpen ParentChunk ckid=RIFF fccType=RMID cksize=000052DA mci.c:191: Test failed: mci open BOSSNOVA.RMI error: 296(40 MCIERR_INVALID_FILE) Its introduction seems unrelated to the rest of that commit and there's no reason given for placing it there, so let's remove the mmioSeek call to fix RMID file handling in MCI. Previous mmioDescend call will set the position to just after the chunk's form type, which is exactly where it should be when calling mmioDescend the second time to find the data chunk. Other parts of Wine code that deal with RIFF files don't seek in analogous situations either. Fixes: 9e1008f13735 ("Removed fixed size array to store specific data") Signed-off-by: Sebastian Krzyszkowiak <dos(a)dosowisko.net> --- dlls/mciseq/mcimidi.c | 1 - 1 file changed, 1 deletion(-) diff --git a/dlls/mciseq/mcimidi.c b/dlls/mciseq/mcimidi.c index 4211ed5ca4a..374f5286b22 100644 --- a/dlls/mciseq/mcimidi.c +++ b/dlls/mciseq/mcimidi.c @@ -717,7 +717,6 @@ static DWORD MIDI_mciOpen(WINE_MCIMIDI* wmm, DWORD dwFlags, LPMCI_OPEN_PARMSW lp if (ckMainRIFF.ckid == FOURCC_RIFF && ckMainRIFF.fccType == mmioFOURCC('R', 'M', 'I', 'D')) { mmckInfo.ckid = mmioFOURCC('d', 'a', 't', 'a'); - mmioSeek(wmm->hFile, ckMainRIFF.dwDataOffset + ((ckMainRIFF.cksize + 1) & ~1), SEEK_SET); if (mmioDescend(wmm->hFile, &mmckInfo, &ckMainRIFF, MMIO_FINDCHUNK) == 0) { TRACE("... is a 'RMID' file\n"); dwOffset = mmckInfo.dwDataOffset; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6696
patch looks reasonable to me... even I don't quite remember why the offending line has been added. (local testing on my remaining .rmi and .mid files show no issues) got also nested exceptions while testing playback of rmi/midi files, will have a look on these crashes -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6696#note_85516
also some of the test failures look suspicious (could be triggered by same issue as I stated above, with either a pop up window showing up, or debugger's). @dos1 would you mind inserting the attached patch into your merge request (before your patch so that we don't break future git bisect) so that we can rerun the tests and see what gives TIA (with attached patch, I longer get the nested exceptions locally) [patch](/uploads/3f55d09eb776d6a7dc5fed3af0557418/patch) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6696#note_85521
participants (3)
-
eric pouech (＠epo) -
Sebastian Krzyszkowiak
-
Sebastian Krzyszkowiak (＠dos1)