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

Issue 646033007: Use the optimal buffer size for the local audio renderer. (Closed)

Created:
6 years, 2 months ago by no longer working on chromium
Modified:
6 years, 1 month ago
CC:
chromium-reviews, posciak+watch_chromium.org, jam, mcasas+watch_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, mkwst+moarreviews-renderer_chromium.org, wjia+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Use the optimal buffer size for the local audio renderer. The issue is that the WebRtcLocalAudioRenderer is using 128 samples as buffer size on Mac, which can screw up the CoreAudio. And it makes perfect sense to use the same buffer size to render both the local and remote audio stream. And we have the same issue on Linux as well. This patch fix the problem on all platforms. BUG=412765, 410258 Committed: https://crrev.com/45bd4345afae9ec2a8676b0a13c9dfbf58f8c32d Cr-Commit-Position: refs/heads/master@{#300700}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+11 lines, -13 lines) Patch
M content/renderer/media/webrtc_audio_renderer.h View 1 chunk +4 lines, -0 lines 0 comments Download
M content/renderer/media/webrtc_audio_renderer.cc View 2 chunks +4 lines, -4 lines 0 comments Download
M content/renderer/media/webrtc_local_audio_renderer.cc View 2 chunks +3 lines, -9 lines 0 comments Download

Messages

Total messages: 10 (3 generated)
no longer working on chromium
Henrik, could you please review? Thanks, SX
6 years, 2 months ago (2014-10-22 14:09:52 UTC) #2
henrika (OOO until Aug 14)
LGTM
6 years, 2 months ago (2014-10-22 14:14:34 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/646033007/1
6 years, 2 months ago (2014-10-22 14:16:40 UTC) #5
commit-bot: I haz the power
Committed patchset #1 (id:1)
6 years, 2 months ago (2014-10-22 17:56:41 UTC) #6
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/45bd4345afae9ec2a8676b0a13c9dfbf58f8c32d Cr-Commit-Position: refs/heads/master@{#300700}
6 years, 2 months ago (2014-10-22 17:57:28 UTC) #7
DaleCurtis
Does this resolve the Hangouts issue discussed on https://codereview.chromium.org/661443002/ ?
6 years, 2 months ago (2014-10-22 18:04:56 UTC) #9
no longer working on chromium
6 years, 1 month ago (2014-10-23 21:04:39 UTC) #10
Message was sent while issue was closed.
On 2014/10/22 18:04:56, DaleCurtis wrote:
> Does this resolve the Hangouts issue discussed on
> https://codereview.chromium.org/661443002/ ?

No, this only affects the use case when a local media stream (capture stream) is
rendered to speaker, since hangout does not render a local media stream, so it
doesn't affect hangout at all.
The hangout issue is related to WebAudio stream, I will soon make a CL to change
the default buffer size from 128 to 256.

Powered by Google App Engine
This is Rietveld 408576698