[Bug 33045] Bunch of Heroes freezing on start at the beginning of the intro video

[wine-bugs at winehq.org]

http://bugs.winehq.org/show_bug.cgi?id=33045

Jörg Höhle <hoehle at users.sourceforge.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |source

--- Comment #3 from Jörg Höhle <hoehle at users.sourceforge.net> 2013-03-01 03:56:22 CST ---
>Is the number of bytes not a multiple of nBlockAlign?
That's it!  Following an intuition, I added -1 to the one line in our
winmm/wave tests "make sure fragment length is a multiple of block size" and
got a deadlock resembling yours.

There are 2 winmm tests that have been on my TODO list for long:
1. Tests with length not a multiple of block size ;-)
2. Tests involving WHDR_BEGIN&ENDLOOP.

I'm convinced that native winmm operates at the level of bytes, not frames. 
That's what I derive from my overflow tests (see bug #23079, comment #4),
performed on w95.  Not having to consider block size also makes chaining ACM
codecs somewhat easier: just operate on a stream of bytes.

We need a fix to the MarkDoneHeaders/PushData pair.  Things to consider:

1. Perform tests with length not a multiple of block size.

2. Investigate what native does with trailing bytes in case of underrun.
  - Throw away?
  - Do they prevent buffer notification, awaiting more bytes from the
    next header to complete a frame?  That is a hairy situation!
  - Do they count in GetPosition?
  Alas, this mostly requires audible tests, not testbot.

3. Make MarkDoneHeaders optional, such that
  waveOutWrite -> BeginPlaying -> PushData(nomark)
  DevicesThreadProc -> PushData(mark&notify)
  IOW, fix bug #TODO.
  Ensure that PushData does not depend on state variables changes performed in
the mark phase.  I've had this change in mind for some time, but it's precisely
the complicated logic in those two functions that prevented it.  I ended up
never reviewing this part of winmm.

4. Document the invariants.
  When is device->loop_start invalid?
  When is device->last invalid?

5. Safe guard against rogue IAC_GetPosition in MarkDone.
Currently it seems that a header can be put into the notification list, yet
still be referenced from device->playing.

6. WHDR_ENDLOOP is queried in 3 places.  Have the code of all those similar,
such that it becomes obvious that they all implement the same behaviour.
  - What about WHDR_ENDLOOP without BEGINLOOP?
  - What about consecutive WHDR_BEGINLOOP?
  - What about BEGINLOOP|ENDLOOP in one header?
  - What about BEGIN ... BEGIN|END ... END?
  - Handle BEGIN ... END ... BEGIN ... END well.

Be careful about loops.  One of my notes is that I suspect MarkDoneHeaders
incorrect with waveOutBreakLoop & several loops, e.g. one already played and
one playing.
There are two traversals of the header list.  One based on GetPosition
(clock_frames), the other based on Get/ReleaseBuffer (queue_frames).  Clearly,
both cannot share the same device->loop_counter.

I like Andrew's idea of a MarkDoneHeaders distinct from PushData.  The code
just needs to step the "done" and "playing" pointers into the header list more
carefully.

-- 
Configure bugmail: http://bugs.winehq.org/userprefs.cgi?tab=email
Do not reply to this email, post in Bugzilla using the
above URL to reply.
------- You are receiving this mail because: -------
You are watching all bug changes.