[PATCH v3 0/4] MR10089: mciavi32: Fix handling of the end position in the play command.
In a certain game from 1996, the end of the logo animation was determined by checking whether the video playback had reached its end. However, Wine's avivideo incorrectly reports the end position as one frame short of the actual length. As a result, the app cannot detect the end of video playback without this change. -- v3: mciavi32: Fix handling of the end position in the step command. mciavi32: Fix handling of the end position in the seek command. mciavi32: Fix handling of the end position in the play command. winmm/tests: Add end position tests for avivideo. https://gitlab.winehq.org/wine/wine/-/merge_requests/10089
From: Akihiro Sagawa <sagawa.aki@gmail.com> --- dlls/winmm/tests/mci.c | 106 ++++++++++++++++++++++++++++++++++++++ dlls/winmm/tests/rsrc.rc | 4 ++ dlls/winmm/tests/test.avi | Bin 0 -> 6376 bytes 3 files changed, 110 insertions(+) create mode 100644 dlls/winmm/tests/test.avi diff --git a/dlls/winmm/tests/mci.c b/dlls/winmm/tests/mci.c index f6654912515..029609d8835 100644 --- a/dlls/winmm/tests/mci.c +++ b/dlls/winmm/tests/mci.c @@ -43,6 +43,8 @@ typedef union { MCI_DGV_WHERE_PARMS where; MCI_DGV_WINDOW_PARMSW win; MCI_GENERIC_PARMS gen; + MCI_PLAY_PARMS play; + MCI_DGV_STEP_PARMS step; } MCI_PARMS_UNION; const char* dbg_mcierr(MCIERROR err) @@ -1763,6 +1765,109 @@ static void test_video_window(void) ok(ret, "Failed to delete %s, error %lu.\n", debugstr_w(filename), GetLastError()); } +static void test_avi_end_position(void) +{ + const WCHAR *filename = load_resource(L"test.avi"); + MCI_PARMS_UNION parm; + DWORD_PTR frames; + MCIDEVICEID id; + MCIERROR err; + BOOL ret; + + parm.dgv_open.lpstrDeviceType = (WCHAR *)L"AVIVideo"; + parm.dgv_open.lpstrElementName = (WCHAR *)filename; + err = mciSendCommandW(0, MCI_OPEN, MCI_OPEN_ELEMENT | MCI_OPEN_TYPE, (DWORD_PTR)&parm); + ok(!err, "Got %s.\n", dbg_mcierr(err)); + id = parm.dgv_open.wDeviceID; + + parm.status.dwItem = MCI_STATUS_TIME_FORMAT; + parm.status.dwReturn = 0xFEEDABAD; + err = mciSendCommandW(id, MCI_STATUS, MCI_STATUS_ITEM, (DWORD_PTR)&parm); + ok(!err,"mciCommand status time format: %s\n", dbg_mcierr(err)); + ok(parm.status.dwReturn == MCI_FORMAT_FRAMES, "status time format: %Id\n", parm.status.dwReturn); + + parm.status.dwItem = MCI_STATUS_LENGTH; + parm.status.dwReturn = 0xFEEDABAD; + err = mciSendCommandW(id, MCI_STATUS, MCI_STATUS_ITEM, (DWORD_PTR)&parm); + ok(!err,"mciCommand status length: %s\n", dbg_mcierr(err)); + ok(parm.status.dwReturn > 0, "status length: %Id\n", parm.status.dwReturn); + frames = (DWORD_PTR)parm.status.dwReturn; + + /* before playing, the position equals to zero */ + parm.status.dwItem = MCI_STATUS_POSITION; + parm.status.dwReturn = 0xFEEDABAD; + err = mciSendCommandW(id, MCI_STATUS, MCI_STATUS_ITEM, (DWORD_PTR)&parm); + ok(!err, "mciCommand status position: %s\n", dbg_mcierr(err)); + ok(parm.status.dwReturn == 0, "Got %Iu, expected 0\n", parm.status.dwReturn); + + /* after playing, the position equals to the length */ + err = mciSendCommandW(id, MCI_PLAY, MCI_WAIT, (DWORD_PTR)&parm); + ok(!err, "mciCommand play: %s\n", dbg_mcierr(err)); + + parm.status.dwItem = MCI_STATUS_POSITION; + parm.status.dwReturn = 0xFEEDABAD; + err = mciSendCommandW(id, MCI_STATUS, MCI_STATUS_ITEM, (DWORD_PTR)&parm); + ok(!err, "mciCommand status position: %s\n", dbg_mcierr(err)); + todo_wine ok(parm.status.dwReturn == frames, "Got %Iu, expected %Iu\n", parm.status.dwReturn, frames); + + /* test "play to" range */ + parm.play.dwTo = frames; + err = mciSendCommandW(id, MCI_PLAY, MCI_TO | MCI_WAIT, (DWORD_PTR)&parm); + ok(!err, "mciCommand play to %lu: %s\n", parm.play.dwTo, dbg_mcierr(err)); + + parm.play.dwTo = frames + 1; + err = mciSendCommandW(id, MCI_PLAY, MCI_TO | MCI_WAIT, (DWORD_PTR)&parm); + todo_wine ok(err == MCIERR_OUTOFRANGE, "mciCommand play to %lu: %s\n", parm.play.dwTo, dbg_mcierr(err)); + + /* test "seek to" range */ + parm.seek.dwTo = frames; + err = mciSendCommandW(id, MCI_SEEK, MCI_TO, (DWORD_PTR)&parm); + todo_wine ok(!err, "mciCommand seek to %lu: %s\n", parm.seek.dwTo, dbg_mcierr(err)); + + parm.seek.dwTo = frames + 1; + err = mciSendCommandW(id, MCI_SEEK, MCI_TO, (DWORD_PTR)&parm); + ok(err == MCIERR_OUTOFRANGE, "mciCommand play to %lu: %s\n", parm.play.dwTo, dbg_mcierr(err)); + + parm.seek.dwTo = 0; + err = mciSendCommandW(id, MCI_SEEK, MCI_TO | MCI_WAIT, (DWORD_PTR)&parm); + ok(!err, "mciCommand seek to %lu: %s\n", parm.seek.dwTo, dbg_mcierr(err)); + + /* the end position equals to the length */ + err = mciSendCommandW(id, MCI_SEEK, MCI_SEEK_TO_END | MCI_WAIT, (DWORD_PTR)&parm); + ok(!err, "mciCommand seek to end: %s\n", dbg_mcierr(err)); + + parm.status.dwItem = MCI_STATUS_POSITION; + parm.status.dwReturn = 0xFEEDABAD; + err = mciSendCommandW(id, MCI_STATUS, MCI_STATUS_ITEM, (DWORD_PTR)&parm); + ok(!err, "mciCommand status position: %s\n", dbg_mcierr(err)); + todo_wine ok(parm.status.dwReturn == frames, "Got %Iu, expected %Iu\n", parm.status.dwReturn, frames); + + /* the start position equals to zero */ + err = mciSendCommandW(id, MCI_SEEK, MCI_SEEK_TO_START | MCI_WAIT, (DWORD_PTR)&parm); + ok(!err, "mciCommand seek to end: %s\n", dbg_mcierr(err)); + + parm.status.dwItem = MCI_STATUS_POSITION; + parm.status.dwReturn = 0xFEEDABAD; + err = mciSendCommandW(id, MCI_STATUS, MCI_STATUS_ITEM, (DWORD_PTR)&parm); + ok(!err, "mciCommand status position: %s\n", dbg_mcierr(err)); + ok(parm.status.dwReturn == 0, "Got %Iu, expected %Iu\n", parm.status.dwReturn, frames); + + /* test "step to" range */ + parm.step.dwFrames = frames; + err = mciSendCommandW(id, MCI_STEP, MCI_DGV_STEP_FRAMES | MCI_TEST, (DWORD_PTR)&parm); + todo_wine ok(!err, "mciCommand step by %lu: %s\n", parm.step.dwFrames, dbg_mcierr(err)); + + parm.step.dwFrames = frames + 1; + err = mciSendCommandW(id, MCI_STEP, MCI_DGV_STEP_FRAMES | MCI_TEST, (DWORD_PTR)&parm); + ok(err == MCIERR_OUTOFRANGE, "mciCommand step by %lu: %s\n", parm.step.dwFrames, dbg_mcierr(err)); + + err = mciSendCommandW(id, MCI_CLOSE, 0, 0); + ok(!err, "Got %s.\n", dbg_mcierr(err)); + + ret = DeleteFileW(filename); + ok(ret, "Failed to delete %s, error %lu.\n", debugstr_w(filename), GetLastError()); +} + START_TEST(mci) { char curdir[MAX_PATH], tmpdir[MAX_PATH]; @@ -1789,6 +1894,7 @@ START_TEST(mci) skip("No output devices available, skipping all output tests\n"); test_video_window(); + test_avi_end_position(); /* Win9X hangs when exiting with something still open. */ err = mciSendStringA("close all", NULL, 0, hwnd); diff --git a/dlls/winmm/tests/rsrc.rc b/dlls/winmm/tests/rsrc.rc index b77f501344e..aeda67e1a6a 100644 --- a/dlls/winmm/tests/rsrc.rc +++ b/dlls/winmm/tests/rsrc.rc @@ -24,6 +24,10 @@ /* @makedep: test.mpg */ test.mpg RCDATA "test.mpg" +/* ffmpeg -f lavfi -i smptebars -t 0.16 -r 24 -f avi -vcodec msvideo1 -pix_fmt pal8 -vf scale=32x24 test.avi */ +/* @makedep: test.avi */ +test.avi RCDATA "test.avi" + /* ffmpeg -f lavfi -i "sine=frequency=1000" -t 0.1 -c:a pcm_s24le test_s24le.wav */ /* @makedep: test_s24le.wav */ test_s24le.wav WAVE "test_s24le.wav" diff --git a/dlls/winmm/tests/test.avi b/dlls/winmm/tests/test.avi new file mode 100644 index 0000000000000000000000000000000000000000..1338aec90f7169aef47fda27b1cbbe43197d5109 GIT binary patch literal 6376 zcmWIYbaQ(k!NA}c=BeQ08609E#K4e|Qk0WemYHF}z`$^55d*`DL<TSr;ACK60kaqw z7{Rmvg8~Bsg9Mn3N`TCIAi%&-TvC)%Tv7x!t1L66*f%)L8CfOB9GHHP*}oVW82<nN z{~xACfk6UfMw$i#0|N-d>|<mQU;vxN22+Wky+ZxGB?J&|H84m?M#zq`MnhmU1V%$( zGz3ONU^E0qLtr!nMnhmU1V%$(Gz3ONU^E0qLI{)<6cxB2c@#AM02^0eW?+GjX~4!N zU~(WDNgJ368f#!-WMIfo$<0a0&B^!y79E)cXv{~7fq}u(&&}U6*e!&Qfq}s%u`JEZ zK+n)j&(Oet0Tfz4m`7&ljQV;s1V(xYfJz80CI*Jw{IX2Ys4Qr-_ie*n!PgC&B>&el zFq~%CD9OO^zrMT_Gy?p;etOvd`hWExntzU<ptb}v3qu14nj5}tU|`^3e9K_YBF4bL zV8L=X@^r&Q$$Qld3?3}%3=SNLEajs2elsxKuP+Z{lUZkBl_3#xRFHQDpA4T2zd~4u zi3bCNBFkM@?HL>bpmLp&L4n~t<2AAWsfrDdSo*<qohe9eDT89ie@n#{kZJ~oA5mAM zu0~yt`f=~^qpOkHhgX>WWZKE*?R?zmCzB-e*UB$Vo7o;PEnpI1{>=1%iGjI1^+6K@ z!)1?+E)2{Mm@fNFba5@UydjyOo4^$!nIM^>o5CGoV9b!f;v?Q5asTn7sgd4?j~huc z|73!hxwTTH`7;wpH_Xh`REB1VSs+o6nWd$Y4<Kg2%w)boYxA_7PZ<4Vl48c<zEV{8 zO>)8RJ_ZI*3IL7y8?@kNXQosbIxs-w1sEV>`k+(;(hG_wHYSJ|j4y=7mxJ;F6Zi91 literal 0 HcmV?d00001 -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10089
From: Akihiro Sagawa <sagawa.aki@gmail.com> --- dlls/mciavi32/mciavi.c | 9 ++++++--- dlls/mciavi32/mmoutput.c | 22 ++++++++++++++-------- dlls/winmm/tests/mci.c | 4 ++-- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/dlls/mciavi32/mciavi.c b/dlls/mciavi32/mciavi.c index a62c791cc9c..a726d63b6d0 100644 --- a/dlls/mciavi32/mciavi.c +++ b/dlls/mciavi32/mciavi.c @@ -570,7 +570,7 @@ static DWORD MCIAVI_mciPlay(UINT wDevID, DWORD dwFlags, LPMCI_PLAY_PARMS lpParms } dwFromFrame = wma->dwCurrVideoFrame; - dwToFrame = wma->dwPlayableVideoFrames - 1; + dwToFrame = wma->dwPlayableVideoFrames; if (dwFlags & MCI_FROM) { dwFromFrame = MCIAVI_ConvertTimeFormatToFrame(wma, lpParms->dwFrom); @@ -578,8 +578,11 @@ static DWORD MCIAVI_mciPlay(UINT wDevID, DWORD dwFlags, LPMCI_PLAY_PARMS lpParms if (dwFlags & MCI_TO) { dwToFrame = MCIAVI_ConvertTimeFormatToFrame(wma, lpParms->dwTo); } - if (dwToFrame >= wma->dwPlayableVideoFrames) - dwToFrame = wma->dwPlayableVideoFrames - 1; + if (dwToFrame > wma->dwPlayableVideoFrames) + { + LeaveCriticalSection(&wma->cs); + return MCIERR_OUTOFRANGE; + } TRACE("Playing from frame=%lu to frame=%lu\n", dwFromFrame, dwToFrame); diff --git a/dlls/mciavi32/mmoutput.c b/dlls/mciavi32/mmoutput.c index dd8f96e1314..1a3945a60cd 100644 --- a/dlls/mciavi32/mmoutput.c +++ b/dlls/mciavi32/mmoutput.c @@ -588,23 +588,29 @@ double MCIAVI_PaintFrame(WINE_MCIAVI* wma, HDC hDC) { void* pBitmapData; LPBITMAPINFO pBitmapInfo; + DWORD frame; if (!hDC || !wma->inbih) return 0; - TRACE("Painting frame %lu (cached %lu)\n", wma->dwCurrVideoFrame, wma->dwCachedFrame); + if (wma->dwCurrVideoFrame >= wma->dwPlayableVideoFrames) + frame = wma->dwPlayableVideoFrames - 1; + else + frame = wma->dwCurrVideoFrame; + + TRACE("Painting frame %lu (cached %lu)\n", frame, wma->dwCachedFrame); - if (wma->dwCurrVideoFrame != wma->dwCachedFrame) + if (frame != wma->dwCachedFrame) { - if (!wma->lpVideoIndex[wma->dwCurrVideoFrame].dwOffset) + if (!wma->lpVideoIndex[frame].dwOffset) return 0; - if (wma->lpVideoIndex[wma->dwCurrVideoFrame].dwSize) + if (wma->lpVideoIndex[frame].dwSize) { - mmioSeek(wma->hFile, wma->lpVideoIndex[wma->dwCurrVideoFrame].dwOffset, SEEK_SET); - mmioRead(wma->hFile, wma->indata, wma->lpVideoIndex[wma->dwCurrVideoFrame].dwSize); + mmioSeek(wma->hFile, wma->lpVideoIndex[frame].dwOffset, SEEK_SET); + mmioRead(wma->hFile, wma->indata, wma->lpVideoIndex[frame].dwSize); - wma->inbih->biSizeImage = wma->lpVideoIndex[wma->dwCurrVideoFrame].dwSize; + wma->inbih->biSizeImage = wma->lpVideoIndex[frame].dwSize; if (wma->hic && ICDecompress(wma->hic, 0, wma->inbih, wma->indata, wma->outbih, wma->outdata) != ICERR_OK) @@ -614,7 +620,7 @@ double MCIAVI_PaintFrame(WINE_MCIAVI* wma, HDC hDC) } } - wma->dwCachedFrame = wma->dwCurrVideoFrame; + wma->dwCachedFrame = frame; } if (wma->hic) { diff --git a/dlls/winmm/tests/mci.c b/dlls/winmm/tests/mci.c index 029609d8835..e9c2b11173d 100644 --- a/dlls/winmm/tests/mci.c +++ b/dlls/winmm/tests/mci.c @@ -1808,7 +1808,7 @@ static void test_avi_end_position(void) parm.status.dwReturn = 0xFEEDABAD; err = mciSendCommandW(id, MCI_STATUS, MCI_STATUS_ITEM, (DWORD_PTR)&parm); ok(!err, "mciCommand status position: %s\n", dbg_mcierr(err)); - todo_wine ok(parm.status.dwReturn == frames, "Got %Iu, expected %Iu\n", parm.status.dwReturn, frames); + ok(parm.status.dwReturn == frames, "Got %Iu, expected %Iu\n", parm.status.dwReturn, frames); /* test "play to" range */ parm.play.dwTo = frames; @@ -1817,7 +1817,7 @@ static void test_avi_end_position(void) parm.play.dwTo = frames + 1; err = mciSendCommandW(id, MCI_PLAY, MCI_TO | MCI_WAIT, (DWORD_PTR)&parm); - todo_wine ok(err == MCIERR_OUTOFRANGE, "mciCommand play to %lu: %s\n", parm.play.dwTo, dbg_mcierr(err)); + ok(err == MCIERR_OUTOFRANGE, "mciCommand play to %lu: %s\n", parm.play.dwTo, dbg_mcierr(err)); /* test "seek to" range */ parm.seek.dwTo = frames; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10089
From: Akihiro Sagawa <sagawa.aki@gmail.com> --- dlls/mciavi32/mciavi.c | 4 ++-- dlls/winmm/tests/mci.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/mciavi32/mciavi.c b/dlls/mciavi32/mciavi.c index a726d63b6d0..ef76243c65f 100644 --- a/dlls/mciavi32/mciavi.c +++ b/dlls/mciavi32/mciavi.c @@ -760,12 +760,12 @@ static DWORD MCIAVI_mciSeek(UINT wDevID, DWORD dwFlags, LPMCI_SEEK_PARMS lpParms if (dwFlags & MCI_TO) { position = MCIAVI_ConvertTimeFormatToFrame(wma, lpParms->dwTo); - if (position >= wma->dwPlayableVideoFrames) + if (position > wma->dwPlayableVideoFrames) return MCIERR_OUTOFRANGE; } else if (dwFlags & MCI_SEEK_TO_START) { position = 0; } else { - position = wma->dwPlayableVideoFrames - 1; + position = wma->dwPlayableVideoFrames; } if (dwFlags & MCI_TEST) return 0; diff --git a/dlls/winmm/tests/mci.c b/dlls/winmm/tests/mci.c index e9c2b11173d..0b49a6e06e8 100644 --- a/dlls/winmm/tests/mci.c +++ b/dlls/winmm/tests/mci.c @@ -1822,7 +1822,7 @@ static void test_avi_end_position(void) /* test "seek to" range */ parm.seek.dwTo = frames; err = mciSendCommandW(id, MCI_SEEK, MCI_TO, (DWORD_PTR)&parm); - todo_wine ok(!err, "mciCommand seek to %lu: %s\n", parm.seek.dwTo, dbg_mcierr(err)); + ok(!err, "mciCommand seek to %lu: %s\n", parm.seek.dwTo, dbg_mcierr(err)); parm.seek.dwTo = frames + 1; err = mciSendCommandW(id, MCI_SEEK, MCI_TO, (DWORD_PTR)&parm); @@ -1840,7 +1840,7 @@ static void test_avi_end_position(void) parm.status.dwReturn = 0xFEEDABAD; err = mciSendCommandW(id, MCI_STATUS, MCI_STATUS_ITEM, (DWORD_PTR)&parm); ok(!err, "mciCommand status position: %s\n", dbg_mcierr(err)); - todo_wine ok(parm.status.dwReturn == frames, "Got %Iu, expected %Iu\n", parm.status.dwReturn, frames); + ok(parm.status.dwReturn == frames, "Got %Iu, expected %Iu\n", parm.status.dwReturn, frames); /* the start position equals to zero */ err = mciSendCommandW(id, MCI_SEEK, MCI_SEEK_TO_START | MCI_WAIT, (DWORD_PTR)&parm); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10089
From: Akihiro Sagawa <sagawa.aki@gmail.com> --- dlls/mciavi32/mciavi.c | 2 +- dlls/winmm/tests/mci.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dlls/mciavi32/mciavi.c b/dlls/mciavi32/mciavi.c index ef76243c65f..5706b60472b 100644 --- a/dlls/mciavi32/mciavi.c +++ b/dlls/mciavi32/mciavi.c @@ -863,7 +863,7 @@ static DWORD MCIAVI_mciStep(UINT wDevID, DWORD dwFlags, LPMCI_DGV_STEP_PARMS lpP if (dwFlags & MCI_DGV_STEP_FRAMES) delta = lpParms->dwFrames; if (dwFlags & MCI_DGV_STEP_REVERSE) delta = -delta; position = wma->dwCurrVideoFrame + delta; - if (position >= wma->dwPlayableVideoFrames) return MCIERR_OUTOFRANGE; + if (position > wma->dwPlayableVideoFrames) return MCIERR_OUTOFRANGE; if (dwFlags & MCI_TEST) return 0; MCIAVI_mciStop(wDevID, MCI_WAIT, NULL); diff --git a/dlls/winmm/tests/mci.c b/dlls/winmm/tests/mci.c index 0b49a6e06e8..efdfbaadcfa 100644 --- a/dlls/winmm/tests/mci.c +++ b/dlls/winmm/tests/mci.c @@ -1855,7 +1855,7 @@ static void test_avi_end_position(void) /* test "step to" range */ parm.step.dwFrames = frames; err = mciSendCommandW(id, MCI_STEP, MCI_DGV_STEP_FRAMES | MCI_TEST, (DWORD_PTR)&parm); - todo_wine ok(!err, "mciCommand step by %lu: %s\n", parm.step.dwFrames, dbg_mcierr(err)); + ok(!err, "mciCommand step by %lu: %s\n", parm.step.dwFrames, dbg_mcierr(err)); parm.step.dwFrames = frames + 1; err = mciSendCommandW(id, MCI_STEP, MCI_DGV_STEP_FRAMES | MCI_TEST, (DWORD_PTR)&parm); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/10089
v3: resolve conflicts in dlls/winmm/tests/rsrc.rc . -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10089#note_130815
This merge request was approved by Huw Davies. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/10089
participants (3)
-
Akihiro Sagawa -
Akihiro Sagawa (＠sgwaki)
-
Huw Davies (＠huw)