mciqtz32: Guarantees that CoInitialize and CoUninitialize run on the same thread.

Aki,

if I understand your critical section well, you serialize all accesses to
the individual player thread. This may be ok for commands that return immediately,
e.g. STATUS, but the MCI knows commands that may take a long time,
in particular PLAY, RECORD and even SEEK.

Did you run a test like this:
CreateTask {
   SetEvent started
   MCICommand(MCI_PLAY, "av_file_that_takes_3_minutes.avi", MCI_WAIT);
 }
Wait(started);
Repeat 10 times {
  MCICommand(MCI_STATUS,MCI_POSITION);
  printf
  MCICommand(MCI_STATUS,MCI_MODE);
  Printf
  Sleep(2s);
}
Wait(thread);

I would expect all MCI commands "STATUS POSITION" etc. to work while another thread is busy playing a file synchronously (using MCI_WAIT).


Furthermore
+static LRESULT MCIQTZ_relayTaskMessage [...]
+    if (WaitForSingleObject(wma->task.done, INFINITE) == WAIT_OBJECT_0)

Are you sure it's ok to block the user thread like this? Shouldn't the code use a loop that also pumps the MS-Windows message queue and dispatches messages? E.g. there may be a need to dispatch REFRESH and PAINT messages?

Did you cross-check what MS-Windows does here?


On a related note, IIRC some wine devices use a more reliable approach to waiting:
WaitForMultiple(2 objects: [task.done, task.threadId], INFINITE);
That way, should the player crash or exit somehow, the main (user) task doesn't hang.

Regards,
	Jörg Höhle

[Akihiro Sagawa]

Hi Jörg,
Thanks for your comments.

On Fri, 29 May 2015 17:50:18 +0200, Jörg Höhle wrote:
> I would expect all MCI commands "STATUS POSITION" etc. to work while
> another thread is busy playing a file synchronously (using MCI_WAIT).
> Good point.  I worried about this situation.  However, recent Windows
> systems don't allow to call MCI commands from another thread.  Therefore,
> this case might not happen.  I also considered to implement a sort of
> corss-thread protection like native, but that breaks compatibilities for
> Win9x and affects all devices.  Because I'm not sure about those effects,
> I left the issue.  Do you know the application which relies on this
> feature?
> Furthermore
> +static LRESULT MCIQTZ_relayTaskMessage [...]
> +    if (WaitForSingleObject(wma->task.done, INFINITE) == WAIT_OBJECT_0)
>
> Are you sure it's ok to block the user thread like this? Shouldn't the code use a loop
> that also pumps the MS-Windows message queue and dispatches messages?
> E.g. there may be a need to dispatch REFRESH and PAINT messages?
> Blocking is not a good idea in general.  I'll update the code to do the
> things more asynchronously.
> Did you cross-check what MS-Windows does here?
> I didn't check Windows behavior at that point.  IMHO, MCI code doesn't
> assume the Window message queue and shouldn't touch them. I'll check the
> behavior later.
> On a related note, IIRC some wine devices use a more reliable approach to waiting:
> WaitForMultiple(2 objects: [task.done, task.threadId], INFINITE);
> That way, should the player crash or exit somehow, the main (user) task doesn't hang.
> Good catch. I'll use more reliable way.

Thanks,
Akihiro Sagawa

Akihiro,
> However, recent Windows systems don't allow to call MCI commands from another thread.
> Could you please elaborate? I understand the Co/UnCo issue, but what about other commands?
> Wouldn't the thread invoking Co/UnCo remain strictly internal to winmm?

At the MCI string command level, all you see are "device" numbers and aliases, so
why shouldn't an app be allowed to send "status x position" from any thread in
the app's address space?
> Do you know the application which relies on this feature?
> I rarely analyzed app's requirements. I usually proceed as follows:
> read available documentation,
> perform many tests, then
> turn a meaningful subset of these into wine test format and
> write driver code that makes Wine nearly undistinguishable from native.

As native behavior varies, I prefer to target the version that was
in use at the time when the driver was mostly used (by commercial apps/games).
For the MCI, this means MS-Windows 3.x + 9X/ME and early 2k/XP.
When you wade through forums, you'll notice that some VB programmers
still use the MCI nowadays, but I saw no complex use cases there. Yet
I'm biased, as I hardly investigated mciqtz usage and mostly cared
about mciwave+midi+cda so far.
> > E.g. there may be a need to dispatch REFRESH and PAINT messages?
> > I didn't check Windows behavior at that point.
> > IMHO, MCI code doesn't assume the Window message queue and shouldn't touch them. 
> > Pumping message queue is pure hypothesis on my side, but it would be
> > valuable to know for sure. I'm not familiar with MS-Windows messaging
> > at all. When I raised the question in wine-devel 2013, IIRC I got no response.

I'm unsure there could be no interaction with the message queue in mciavi
and mciqtz. After all, there are MCI_*_HWND command options.


Some other area in need of investigation is timing. When or how fast
does a function return? For instance, there are entries in bugzilla
related to the time it takes to close and reopen winmm.
https://bugs.winehq.org/show_bug.cgi?id=28413

On a related side, at what time are callbacks invoked (or quartz pins)?
Fortunately, the MCI knows no callbacks (lesson learned), but they may
raise their ugly head again via the MCI-Quartz bridge.

As many (old?) apps are single-threaded, so I was told, the duration
of function calls matters, allowing the app to return to its message
queue fast. The order of calls through pins matters too, alas.

For instance, I've wondered whether it would be valuable to send PLAY,
PAUSE and STOP asynchronously, so as to return to the caller fast.
The driver could still have internal logic that would let it accept
the next command only after the current one is processed, so it's
still strictly serial. IOW there's a pipe of length 1 in between.
The difference is that the caller app is free to do other things
for as long as it doesn't issue commands to fast in a row.
I've observed evidence of some async. behavior with at least one
of the MCI drivers (mcicda perhaps, or mcimidi, not sure any more):
not all errors that PLAY would meet were returned immediately upon
command invocation (IIRC there were a few corner cases along
PLAY FROM 100 to 100).
> From an implementation POV, this requires some processing (error checking,
> e.g. refuse PLAY FROM 100 TO 50) before sending the async. message,
> and checking the rest in the async. driver. That's quite different from
> your current "forward immediately" patch. But that's just an idea.

Regards,
	Jörg Höhle

[Akihiro Sagawa]

Hi Jörg,

On Tue, 2 Jun 2015 14:04:48 +0200, Joerg-Cyril.Hoehle wrote:
> > However, recent Windows systems don't allow to call MCI commands from another thread.
> > Could you please elaborate? I understand the Co/UnCo issue, but what about other commands?
> > Wouldn't the thread invoking Co/UnCo remain strictly internal to winmm?
>
> At the MCI string command level, all you see are "device" numbers and aliases, so
> why shouldn't an app be allowed to send "status x position" from any thread in
> the app's address space?

Essentially, it should.  But, please see
https://testbot.winehq.org/JobDetails.pl?Key=13867 and its patch.
In the test, it opens tempfile.wav with alias mysound in mci_open_thread
and plays mysound in mci_another_thread.  It also expects the latter
attempt is failed with MCIERR_INVALID_DEVICE_NAME.
In the main thread, mciGetDeviceID's return value is zero which means an
error.  This indicates the device doesn't exist in winmm level.
Finally, it succeeds to open a file with the same alias in main thread.
The result shows there are no errors at these points.

To wrap up, "device" and alias is not visible between threads.
Among testbot, this is true. Additionally, my test shows it's true for
Windows NT 4, but false for Windows 98.
I'll continue investigating old Windows behavior.

Regards,
Akihiro Sagawa

Hi,

Akihiro Sagawa wrote:
> > > However, recent Windows systems don't allow to call MCI commands from another thread.
> > > In the test, it opens tempfile.wav with alias mysound in mci_open_thread and plays mysound in
> > > mci_another_thread.
> To wrap up, "device" and alias is not visible between threads.
> Among testbot, this is true. Additionally, my test shows it's true for Windows NT 4, but false for Windows 98.

Good catch. This is news to me. Long ago I observed that devices are not visible
across processes (IIRC I opened 2 DOS command windows on a w95 system) but
had no reason to doubt the Wine thread visibility code that I saw.

"Invisibility across threads" confirmed on a real w2k machine (not doubting your test
result, it's just so surprising). I'll test a w95 machine another time. Note that
I doubt that the winmm mci test still works on w95. IIRC, "play mine.wav"
only plays once on a w95 box, because the w95 MCI system appears to switch to C:
afterwards. I can't remember whether the test works if you start in C:.

Older winmm mci tests in git worked on w95, but as I added more tests, I choose not
to care about w95 behaviour and targeted w98 instead, which behaved much better.
I vaguely remember that it was easy to cause w95 to hang with added mci/winmm tests,
whereas w98 and ME worked well. So I defined w98 as the ideal platform for ancient games.
> but false for Windows 98
> Hmm, I really don't know whether Wine should follow w2k/wxp/w7 behaviour here
> (still assuming that it's old games that use the MCI). I'm really
> surprised to learn that MS has changed the behavior that radically.
> OTOH, perhaps there's no issue? W3.x and w9x apps would never(?) use threads,
> (the "cooperative multitasking experience" mind set) so they won't notice a change?

Regards,
	Jörg Höhle