http://bugs.winehq.org/show_bug.cgi?id=29299
Bug #: 29299 Summary: Lords of the Realm 2: in-game videos missing audio Product: Wine Version: 1.3.32 Platform: x86 URL: http://www.sierrahelp.com/Files/Extras/Demos/Lords2Dem o.zip OS/Version: Linux Status: NEW Keywords: download, regression Severity: minor Priority: P2 Component: winmm&mci AssignedTo: wine-bugs@winehq.org ReportedBy: gyebro69@gmail.com CC: hoehle@users.sourceforge.net Classification: Unclassified Regression SHA1: 1cde966c35dcb0b5dc7bc5a6c42b788f6d8f561c
Created attachment 37893 --> http://bugs.winehq.org/attachment.cgi?id=37893 7zipped +tid,+winmm,+mmdevapi,+alsa,+dsound,+mci debug log (uncompressed 1.4 MB)
In-game videos are missing audio in LotR 2. The videos are playing silently. Other in-game sounds (stored in .wav files) are playing correctly. Videos are encoded with the Smacker codec.
Audio worked correctly during video playback in 1.3.31:
1cde966c35dcb0b5dc7bc5a6c42b788f6d8f561c is the first bad commit commit 1cde966c35dcb0b5dc7bc5a6c42b788f6d8f561c Author: Jörg Höhle hoehle@users.sourceforge.net Date: Mon Sep 19 14:30:58 2011 +0200
winmm: Never write silence, mmdevapi must handle underruns.
:040000 040000 442f9faf6718ced2ee637c2b040d4f04b8bb40bb 99757ff1ef66690b3572e6337349900a8ab0b495 M dlls
The patch can be reverted on current git (wine-1.3.34-202-gb0f704d), and that fixed the problem. This commit is also mentioned in bug #29056, although it causes freezes there.
The demo contains only 2 short videos (bat_los1.smk and bat_win1.smk) which are playing when you either win or loose a game. To reach to that point in the game requires several minutes of gameplay, so here I'll give you a different approach to reproduce the problem with the demo.
1. Unpack and install the demo. Use Wine's full-screen mode for installation, and do not perform system test when the installer asks for it. 2. Now that the demo is installed you need the RAD Video Tools utility, including a smacker video player. Download and install RAD Video Tools: http://www.radgametools.com/down/Bink/RADTools.exe 3. Locate the 2 video files in the demo directory (BAT_LOS1.SMK and BAT_WIN1.SMK) and play either of them by using the smacker player application: wine smackplw.exe BAT_WIN1.SMK. You can see the videos playing but cannot hear audio.
Fedora 16 x86 Alsa 1.0.24 PA is not running
http://bugs.winehq.org/show_bug.cgi?id=29299
GyB gyebro69@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- URL|http://www.sierrahelp.com/F |http://www.sierrahelp.com/M |iles/Extras/Demos/Lords2Dem |isc/Demos.html |o.zip |
http://bugs.winehq.org/show_bug.cgi?id=29299
--- Comment #1 from Jörg Höhle hoehle@users.sourceforge.net 2011-12-12 06:53:17 CST --- This is a duplicate of the "ALSA doesn't start when fed less than a period worth of samples" bug, which was shadowed by writing silence.
The first waveOutOpen in your logs writes 4 samples @11025x8x1, the second 2 samples @11025x8x2 that are never acknowledged. My recent GetPosition patch discussed in bug #28723 doesn't help when ALSA is not starting. The possible "if delay < period_size then bump position to maximum" enhancement might help with winmm, but I think it's not GetPosition's job to detect a hanging ALSA. That should be the job of the periodic callback.
I'm unsure whether bug #29056 is the root "ALSA doesn't start" bug because of the confusing reports involving pasuspender there.
http://bugs.winehq.org/show_bug.cgi?id=29299
Jörg Höhle hoehle@users.sourceforge.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Component|winmm&mci |winealsa.drv Severity|minor |major
--- Comment #2 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-10 13:57:24 CST --- Failure to play short samples because ALSA won't start is a major bug known to cause hanging apps, as in bug #26878 and bug #29056.
http://bugs.winehq.org/show_bug.cgi?id=29299
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |austinenglish@gmail.com
--- Comment #3 from Austin English austinenglish@gmail.com 2012-01-16 11:09:43 CST --- *** Bug 29608 has been marked as a duplicate of this bug. ***
http://bugs.winehq.org/show_bug.cgi?id=29299
Austin English austinenglish@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Blocks| |19531
http://bugs.winehq.org/show_bug.cgi?id=29299
Jörg Höhle hoehle@users.sourceforge.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |rosslagerwall@gmail.com
--- Comment #4 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-18 00:35:50 CST --- *** Bug 29056 has been marked as a duplicate of this bug. ***
http://bugs.winehq.org/show_bug.cgi?id=29299
Jörg Höhle hoehle@users.sourceforge.net changed:
What |Removed |Added ---------------------------------------------------------------------------- Target Milestone|--- |1.4.0
--- Comment #5 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-18 01:28:53 CST --- Attachment #37441 to duplicate bug #29056 is a small test program. Or you simply test the MCI Sound command after configuring your system sounds as bug #21277 explains, since winmm:sndPlaySound is affected too -- hence winecfg's "Test sound" button.
http://bugs.winehq.org/show_bug.cgi?id=29299
--- Comment #6 from Andrew Eikum aeikum@codeweavers.com 2012-01-23 11:32:05 CST --- Created attachment 38513 --> http://bugs.winehq.org/attachment.cgi?id=38513 Pre- and post-pads for winealsa
Here's two simple patches which fix both the tiny amount of data problem and the last period not full problem. They're pretty obvious: if there won't be a full period in ALSA before we write, we pre-pad with silence. If there is no data left to write after writing and there isn't a multiple of a full ALSA period in the ALSA buffer, then we write silence to fill out that last period.
I'm concerned about how the second patch affects nearly-empty buffer situations like Bug 28723. I think it should work, but it makes us slightly less tolerant of very low data scenarios (or does it? perhaps ALSA will glitch anyway as it pulls that half-period).
Perhaps I should add TRACEs to these new codepaths...
http://bugs.winehq.org/show_bug.cgi?id=29299
Andrew Eikum aeikum@codeweavers.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |aeikum@codeweavers.com
--- Comment #7 from Andrew Eikum aeikum@codeweavers.com 2012-01-23 11:45:21 CST --- In the first patch, I think the line should be:
+ UINT32 s_frames = This->alsa_period_frames - This->held_frames - in_alsa;
http://bugs.winehq.org/show_bug.cgi?id=29299
--- Comment #8 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-23 16:01:31 CST --- I'll need more time to think, but one thing is definitively wrong with the second patch. The callback is never allowed to pad more than one mmdevapi period worth of frames, because the next callback could see them supplied, hence the patch will cause abnormal effects when ALSA period > mmdevapi period.
Also, you should need no HeapAlloc. Prealloc temp_buffer with max(ALSA,mmdevapi)period (also good for capture). If (This->getbuf_last>= 0) use temp_buffer otherwise there's room in the normal one.
http://bugs.winehq.org/show_bug.cgi?id=29299
--- Comment #9 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-24 05:40:27 CST --- Native's precondition to append silence would be: if (initially_held_frames (==held_frames+written) < mmdevapi_period)
However, ours is much more complicated because we may decrease padding by more than one mmdevapi period. Furthermore, we need to handle cases where a) alsa_period < mmdevapi_period b) alsa_period = mmdevapi_period c) alsa_period > mmdevapi_period
The scenario Release(30ms); Sleep(25ms); Release(more) is perfectly valid. We must not append silence even though our first callback write will move the 3 periods outside the sight of GCP, yielding held_frames == 0. I still need more time to think through the exact condition. There's also that if (held_frames == 0) early return that will get modified.
The lead-in patch is not quite correct. First, the comment is wrong. When we'll have a proper lead-out, its padding will have ALSA start. We don't need the lead-in for that. I invented the lead-in (or its equivalent, the start_threshold) solely to guarantee that ALSA has enough frames to play even in the XAudio2/Rage scenario.
The amount of lead-in frames is not one alsa period. I can't remember whether I explained the threshold = ALSA_period + mmdevapi_period + safety:
+ mmdevapi_period because at the end of the "buffer processing" period, we still want ALSA to have enough data; + safety(=3-5ms) because our next period callback may come a little late; + ALSA_period because we don't know exactly, but I assume ALSA feels better when it has at least one period in its buffers. For instance, dmix appears to operate on period-size chunks.
That is the minimal amount of frames that ALSA should start with. This is what I want hidden beneath GCP=0. When that much is hidden, ALSA will bear being fed one mmdevapi period of frames at every period callback, i.e. the XAudio2/Rage scenario without underrun (well, disregarding clock shift and timer differences).
You are right that an explicit lead-in is easier to handle than an ALSA start_threshold larger than mmdevapi_period. It's also easier to apply to OSS which knows no such condition.
The lead-in precondition seems to be: if (in_alsa == 0 // does that cover the recover from underrun case? && initial_held_frames < threshold)
http://bugs.winehq.org/show_bug.cgi?id=29299
--- Comment #10 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-25 08:48:35 CST --- Created attachment 38549 --> http://bugs.winehq.org/attachment.cgi?id=38549 winealsa silence lead-in + render "small writes" test
This is my (non final) stab at the lead-in issue, introducing the safety threshold that I talked about. Works quite well with ALSA/dmix. I've also written an XAudio2 underrun test case.
http://bugs.winehq.org/show_bug.cgi?id=29299
--- Comment #11 from Andrew Eikum aeikum@codeweavers.com 2012-01-25 13:58:31 CST --- Nice work, Jörg! I tested with your patch and it seems to work well. It fixes a simple deadlock test case that I put together. The tests you included pass both with and without the patch on my computer, so I wasn't able to observe anything there.
You can squash the first two patches together, if you like.
+ This->bufsize_frames = MulDiv(duration, fmt->nSamplesPerSec, 10000000); + /* Deal weird situations were ALSA period (50ms) > mmdevapi buffer (10ms) */ +
Why'd you move this? The comment might have been supposed to explain it, but I can't quite understand what you meant there... what's weird about that situation, and what are we doing to deal [with] it?
+ /* don't write if the buffer is already locked in wrap-around mode */
Maybe "s/buffer/tmp buffer/" for clarity.
http://bugs.winehq.org/show_bug.cgi?id=29299
--- Comment #12 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-27 07:47:23 CST --- For the submitted patch, I reverted to using HeapAlloc like you did, because the lock-free design I have in mind won't allow using the tmp_buffer.
This->bufsize_frames = ... was moved based on the idea that it's only after ALSA is set that mmdevapi's definitive buffer shall be decided. Do you really consider it sane when mmdevapi < ALSA buffer or even ALSA period? I think it'll work, but still...
Béla, please test my patch with all related bugs and apps you know, e.g. bug #26878. It may or may not help, as I wrote when submitting: "The lead-in may solve half the bugs about apps hanging when writing few samples. The other half depends on the lead-out, i.e. both GetCurrentPadding and GetPosition reaching the sum of Release'd frames."
http://bugs.winehq.org/show_bug.cgi?id=29299
GyB gyebro69@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed by SHA1| |7c52a257fb74c21e59318becf70 | |dacffa5f9d720 Status|NEW |RESOLVED Resolution| |FIXED
--- Comment #13 from GyB gyebro69@gmail.com 2012-01-27 12:53:26 CST --- The patch has been committed as http://source.winehq.org/git/wine.git/commit/7c52a257fb74c21e59318becf70dacf...
The audio problem has been resolved in Lords of the Realm 2. This commit also fixes an issue with Space Empires:Star Fury. The game hung when certain (short duration) wav files were playing.
http://bugs.winehq.org/show_bug.cgi?id=29299
Alexandre Julliard julliard@winehq.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #14 from Alexandre Julliard julliard@winehq.org 2012-01-27 14:17:55 CST --- Closing bugs fixed in 1.4-rc1.
http://bugs.winehq.org/show_bug.cgi?id=29299
--- Comment #15 from Jörg Höhle hoehle@users.sourceforge.net 2012-01-28 12:07:28 CST --- Andrew,
You can squash the first two patches together, if you like.
git-rebase --interactive + squash merged the 2 patches using your name. That's how it's in git now, so you'll get the fame and the regressions, if ever :-)
https://bugs.winehq.org/show_bug.cgi?id=29299
Anastasius Focht focht@gmx.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |focht@gmx.net URL|http://www.sierrahelp.com/M |https://archive.org/downloa |isc/Demos.html |d/LordsoftheRealmII_1020/lo | |rddemo.zip
-
wine-bugs＠winehq.org -
WineHQ Bugzilla