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

wine-bugs at winehq.org wine-bugs at winehq.org
Sat Mar 9 04:31:10 CST 2013


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

--- Comment #18 from Jörg Höhle <hoehle at users.sourceforge.net> 2013-03-09 04:31:10 CST ---
Created attachment 43862
  --> http://bugs.winehq.org/attachment.cgi?id=43862
patch: Fix notification when recording using MSACM.

The data is equally consistent with
 samples = bytes * nSamplesPerSec / nAvgBytesPerSec; // not MulDiv

Your patch looks ok now, yet I still have a suggestion.  In WOD_PushData, it's
important that both the while and for loop compute the same number of frames. 
To ease the job of your poor reviewer, it would be nice to have both use the
same function, e.g. WINMM_HeaderLenBytes (BTW, I don't like having three tiny
functions do almost the same thing).

Here's a patch to fix notification in WID_PullACMData atop your patch.  I plan
to rebase it against 1.5.25 and submit it on Monday, so that it does not come
in the middle of your patch series.

What still needs fixing?
- GetPosition returns bogus numbers with ACM codecs, e.g.
wave.c:1626: Test failed: after position: 15655 samples
wave.c:1632: Test failed: after position: 4007680 bytes - not * 256

- Think about WHDR_ENDLOOP without preceding BEGIN.
  Wine  will hang or crash.

- MarkDoneHeaders is still bogus w.r.t. LOOP, see comment #3

- WID_PullACMData looks incomplete.  It starts with checking
  device->acm_hdr as if it could persist from a prior invocation,
  however it frees it when returning normally.  Thus it should remain
  local to the function, with device->acm_offs.  In error
  situations, the current code may break, because it may see
  device->acm_hdr.cbDstLength != 0 on entry from a prior
  acmStreamConvert MMSYSERR_* return.
  Hmm, now that my patch eliminated the tail call, it could as
  well fix that.

- WID_PullACMData needs better error handling. If acmStreamConvert
  returns an error, the IACaptureClient buffer remains locked.

- In WID_PullACMData, the queue->lpNext condition looks bogus in the
  light of IMA_ADPCM's 256 bytes block size.
  More generally, PullACMData needs a redesign.  What to do when
  srcLengthUsed < packet_frames ?!?
  I.e. when the mmdevapi packet length does not match the codec's blocksize,
  e.g. using 10ms packets while IMA_ADPCM likely needs multiples of 256 bytes.
    - We should not throw away recorded bytes given enough buffers.
    - mmdevapi does not accept retrieving less than one packet...
  Ouch, that's enough material for a separate bug report.

-- 
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.


More information about the wine-bugs mailing list