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

Issue 13390010: Fix a couple bugs with OSX audio buffer size selection. (Closed)

Created:
7 years, 8 months ago by DaleCurtis
Modified:
7 years, 8 months ago
CC:
chromium-reviews, feature-media-reviews_chromium.org, sail+watch_chromium.org
Visibility:
Public.

Description

Fix a couple bugs with OSX audio buffer size selection. 1. Device changes may cause an already in process stream creation to use the wrong buffer size => CHECK() failure. 2. Low latency stream Open() failure causes AudioOutputResampler to fall back to a high latency buffer size, which will cause a stream to be created with the wrong buffer size => CHECK() failure. Fixing 2 has the added advantage of removing an extra useless hop between stream Open() failure and switching to a fake audio output device. BUG=225249 TEST=audio works. Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=192946

Patch Set 1 #

Patch Set 2 : Fix unused errors. #

Patch Set 3 : Fix unittests. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+42 lines, -16 lines) Patch
M media/audio/audio_output_proxy_unittest.cc View 1 2 2 chunks +16 lines, -2 lines 0 comments Download
M media/audio/audio_output_resampler.cc View 1 3 chunks +12 lines, -3 lines 0 comments Download
M media/audio/mac/audio_low_latency_output_mac.cc View 7 chunks +14 lines, -11 lines 0 comments Download

Messages

Total messages: 9 (0 generated)
DaleCurtis
7 years, 8 months ago (2013-04-01 20:38:52 UTC) #1
Chris Rogers
lgtm
7 years, 8 months ago (2013-04-01 21:13:51 UTC) #2
DaleCurtis
scherkus: ping?
7 years, 8 months ago (2013-04-05 23:54:03 UTC) #3
scherkus (not reviewing)
lgtm
7 years, 8 months ago (2013-04-08 14:27:48 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/13390010/1
7 years, 8 months ago (2013-04-08 16:54:32 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/13390010/10002
7 years, 8 months ago (2013-04-08 17:04:12 UTC) #6
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
7 years, 8 months ago (2013-04-08 17:11:40 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/dalecurtis@chromium.org/13390010/7004
7 years, 8 months ago (2013-04-08 18:53:30 UTC) #8
commit-bot: I haz the power
7 years, 8 months ago (2013-04-08 23:28:38 UTC) #9
Message was sent while issue was closed.
Change committed as 192946

Powered by Google App Engine
This is Rietveld 408576698