Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(106)

Issue 8753001: Revert 112147 - Did not get LGTM from all reviewers. (Closed)

Created:
9 years ago by enal
Modified:
9 years ago
Reviewers:
enal
CC:
chromium-reviews, hclam+watch_chromium.org, ddorwin+watch_chromium.org, fischman+watch_chromium.org, acolwell+watch_chromium.org, annacc+watch_chromium.org, vrk (LEFT CHROMIUM), scherkus (not reviewing), ihf+watch_chromium.org
Visibility:
Public.

Description

Revert 112147 - Did not get LGTM from all reviewers. Change the way we are sending audio data to driver when using WaveOut API. Before, we were "feeding" the driver in the callback when OS returned audio buffer to us. MSDN disallows that, but We were avoiding deadlock when stopping the stream by using some clever tricks. Unfortunately, exactly the same deadlock happens when Windows were draining audio stream when switching the device, and our tricks did not help, as we were not controlling exact timing. Fix is to separate receiving freed buffer and "feeding" audio driver. Now we are using CALLBACK_EVENT wave out mode in which Windows signal event when buffer runs out of data, and using RegisterWaitForSingleObject() so our callback is called by one of thread pool threads. Stopping of playback became slower because now it is synchronous. If that ever become a problem we can use singleton that keeps track of the playing audio streams, than stopping can become instaneous, at a cost of more complex checks in the callback. Unfortunately, no automatic test, as there is no documented way to programmatically switch default audio device on Windows. People were able to figure it out (see http://www.daveamenta.com/2011-05/programmatically-or-command-line-change-the-default-sound-playback-device-in-windows-7/), but there is no guarantee it will continue to work in Win8, or even in next Service Pack for Win7. BUG=41036 TEST=Play any HTML5 audio/video on Windows and change default audio device TEST=using "Control Panel | Sound". There should be no hang, you should hear TEST=audio from the newly chosen device. Review URL: http://codereview.chromium.org/8591028 TBR=enal@chromium.org Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=112233

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+110 lines, -160 lines) Patch
M media/audio/audio_util.cc View 1 chunk +0 lines, -5 lines 0 comments Download
M media/audio/win/waveout_output_win.h View 4 chunks +20 lines, -29 lines 0 comments Download
M media/audio/win/waveout_output_win.cc View 9 chunks +90 lines, -126 lines 0 comments Download

Messages

Total messages: 1 (0 generated)
enal
9 years ago (2011-11-30 16:21:36 UTC) #1

          

Powered by Google App Engine
This is Rietveld 408576698