Increase Windows XP hardware buffer size to 4096. (issue 11309015)

33 views
Skip to first unread message

dalec...@chromium.org

unread,
Oct 26, 2012, 9:16:42 PM10/26/12
to sche...@chromium.org, cro...@google.com, hen...@chromium.org, viettr...@chromium.org, chromium...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, j...@chromium.org, feature-me...@chromium.org
Reviewers: scherkus, Chris Rogers, henrika, viettrungluu,

Message:
scherkus and I had time to do some testing on his XP laptop with the latest
canary and our various audio clients and found that the 2k buffer size we
use on
XP for low latency clients led to glitching.

Switching this to 4k resolved the problem and doesn't seem to add any
noticeable
overhead in the demos we've tested (html5, flash youtube, iwebdj, and
webaudio).
The WebRTC demo sadly wouldn't connect. Henrika, do you have another good
demo
we can check out?

We're able to use 4k on XP w/ 2k coming from the renderer due to the
blocking
shared memory on Windows.

What does everyone think?

Description:
Increase Windows XP hardware buffer size to 4096.

In local testing with XP we noticed WebAudio and HTML5 suffer from
clicking with the "low latency" 2048 buffer size.

This CL also adds a --audio-buffer-size parameter which we can use
to have users tweak in the field and report results back.

BUG=none
TEST=XP!


Please review this at https://codereview.chromium.org/11309015/

SVN Base: http://git.chromium.org/chromium/src.git@master

Affected files:
M content/browser/renderer_host/render_process_host_impl.cc
M media/audio/audio_util.cc
M media/base/media_switches.h
M media/base/media_switches.cc


Index: content/browser/renderer_host/render_process_host_impl.cc
diff --git a/content/browser/renderer_host/render_process_host_impl.cc
b/content/browser/renderer_host/render_process_host_impl.cc
index
5e470e08d2459aed3ea544bc9a7711c4d3f454e4..37536467e1cf3756d6857bc76c79b6552e1bf604
100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -746,6 +746,7 @@ void
RenderProcessHostImpl::PropagateBrowserCommandLineToRenderer(
static const char* const kSwitchNames[] = {
// We propagate the Chrome Frame command line here as well in case the
// renderer is not run in the sandbox.
+ switches::kAudioBufferSize,
switches::kAuditAllHandles,
switches::kAuditHandles,
switches::kChromeFrame,
Index: media/audio/audio_util.cc
diff --git a/media/audio/audio_util.cc b/media/audio/audio_util.cc
index
8f05410d02980e014311cd3b71214cfbc034ea86..7ab32c540c0791e79cabc846dfeceba52484b50d
100644
--- a/media/audio/audio_util.cc
+++ b/media/audio/audio_util.cc
@@ -19,6 +19,7 @@

#include "base/basictypes.h"
#include "base/logging.h"
+#include "base/string_number_conversions.h"
#include "base/time.h"
#include "media/audio/audio_parameters.h"
#include "media/base/audio_bus.h"
@@ -39,6 +40,21 @@

namespace media {

+// Returns user buffer size as specified on the command line or 0 if no
buffer
+// size has been specified.
+static int GetUserBufferSize() {
+ const CommandLine* cmd_line = CommandLine::ForCurrentProcess();
+ int buffer_size = 0;
+ std::string buffer_size_str(cmd_line->GetSwitchValueASCII(
+ switches::kAudioBufferSize));
+ if (!buffer_size_str.empty() &&
+ base::StringToInt(buffer_size_str, &buffer_size) && buffer_size > 0)
{
+ return buffer_size;
+ }
+
+ return 0;
+}
+
// TODO(fbarchard): Convert to intrinsics for better efficiency.
template<class Fixed>
static int ScaleChannel(int channel, int volume) {
@@ -225,6 +241,10 @@ int GetAudioInputHardwareSampleRate(const std::string&
device_id) {
}

size_t GetAudioHardwareBufferSize() {
+ int user_buffer_size = GetUserBufferSize();
+ if (user_buffer_size)
+ return user_buffer_size;
+
// The sizes here were determined by experimentation and are roughly
// the lowest value (for low latency) that still allowed glitch-free
// audio under high loads.
@@ -236,7 +256,7 @@ size_t GetAudioHardwareBufferSize() {
return 128;
#elif defined(OS_WIN)
// Buffer size to use when a proper size can't be determined from the
system.
- static const int kFallbackBufferSize = 2048;
+ static const int kFallbackBufferSize = 4096;

if (!IsWASAPISupported()) {
// Fall back to Windows Wave implementation on Windows XP or lower
@@ -314,6 +334,10 @@ ChannelLayout GetAudioInputHardwareChannelLayout(const
std::string& device_id) {
// Computes a buffer size based on the given |sample_rate|. Must be used in
// conjunction with AUDIO_PCM_LINEAR.
size_t GetHighLatencyOutputBufferSize(int sample_rate) {
+ int user_buffer_size = GetUserBufferSize();
+ if (user_buffer_size)
+ return user_buffer_size;
+
// TODO(vrk/crogers): The buffer sizes that this function computes is
probably
// overly conservative. However, reducing the buffer size to 2048-8192
bytes
// caused crbug.com/108396. This computation should be revisited while
making
Index: media/base/media_switches.cc
diff --git a/media/base/media_switches.cc b/media/base/media_switches.cc
index
62f0c3945f4c7490e8f598e48ee29282c2515be5..2c0ef68acfffce5327420ca76e44137dca5aad94
100644
--- a/media/base/media_switches.cc
+++ b/media/base/media_switches.cc
@@ -6,6 +6,9 @@

namespace switches {

+// Allow users to specify a custom buffer size for debugging purpose.
+const char kAudioBufferSize[] = "audio-buffer-size";
+
#if defined(OS_LINUX) || defined(OS_FREEBSD) || defined(OS_SOLARIS)
// The Alsa device to use when opening an audio stream.
const char kAlsaOutputDevice[] = "alsa-output-device";
Index: media/base/media_switches.h
diff --git a/media/base/media_switches.h b/media/base/media_switches.h
index
744e191cb6acc092ca8aee5ca289a879f31f9f52..fde744c10cfd0545f39734fe079ccbd01669d136
100644
--- a/media/base/media_switches.h
+++ b/media/base/media_switches.h
@@ -17,6 +17,8 @@ extern const char kAlsaOutputDevice[];
extern const char kAlsaInputDevice[];
#endif

+MEDIA_EXPORT extern const char kAudioBufferSize[];
+
#if defined(USE_CRAS)
MEDIA_EXPORT extern const char kUseCras[];
#endif


hen...@chromium.org

unread,
Oct 27, 2012, 4:46:55 AM10/27/12
to dalec...@chromium.org, sche...@chromium.org, cro...@google.com, viettr...@chromium.org, chromium...@chromium.org, joi+watc...@chromium.org, dari...@chromium.org, j...@chromium.org, feature-me...@chromium.org
Perhaps I am blind but I can't see that you are increasing the buffer size
in
this CL.

If that is what it takes, then I am fine with it. It is possible to reduce
the
delay on XP but it requires a totally new implementation and I don't think
we
have resources for that now.

Other demos: try pc1.html at https://webrtc-demos.appspot.com/

https://codereview.chromium.org/11309015/
Reply all
Reply to author
Forward
0 new messages