Commit 0650f24c authored by hubbe's avatar hubbe Committed by Commit bot

Use audio shifter instead of a fifo for local mediastream playback.

There are two reasons for this change:

1. The current code path doesn't work correctly if the media stream and the output devices clocks don't match up *exactly*. Underruns/lipsync issues can occur.

2. This allows for audio to be explicitly buffered for some period of time, which I plan to utilize in the cast_streaming receiver code.

Currently this code path is not often used, since it only happens when you get a media stream from getUserMedia and plug it into a media player without going through webrtc.

Review URL: https://codereview.chromium.org/856843002

Cr-Commit-Position: refs/heads/master@{#313193}
parent 37489c3c
...@@ -16,7 +16,7 @@ ...@@ -16,7 +16,7 @@
#include "content/renderer/render_frame_impl.h" #include "content/renderer/render_frame_impl.h"
#include "media/audio/audio_output_device.h" #include "media/audio/audio_output_device.h"
#include "media/base/audio_bus.h" #include "media/base/audio_bus.h"
#include "media/base/audio_fifo.h" #include "media/base/audio_shifter.h"
namespace content { namespace content {
...@@ -36,21 +36,15 @@ int WebRtcLocalAudioRenderer::Render( ...@@ -36,21 +36,15 @@ int WebRtcLocalAudioRenderer::Render(
TRACE_EVENT0("audio", "WebRtcLocalAudioRenderer::Render"); TRACE_EVENT0("audio", "WebRtcLocalAudioRenderer::Render");
base::AutoLock auto_lock(thread_lock_); base::AutoLock auto_lock(thread_lock_);
if (!playing_ || !volume_ || !loopback_fifo_) { if (!playing_ || !volume_ || !audio_shifter_) {
audio_bus->Zero(); audio_bus->Zero();
return 0; return 0;
} }
// Provide data by reading from the FIFO if the FIFO contains enough audio_shifter_->Pull(
// to fulfill the request. audio_bus,
if (loopback_fifo_->frames() >= audio_bus->frames()) { base::TimeTicks::Now() -
loopback_fifo_->Consume(audio_bus, 0, audio_bus->frames()); base::TimeDelta::FromMilliseconds(audio_delay_milliseconds));
} else {
audio_bus->Zero();
// This warning is perfectly safe if it happens for the first audio
// frames. It should not happen in a steady-state mode.
DVLOG(2) << "loopback FIFO is not full enough yet";
}
return audio_bus->frames(); return audio_bus->frames();
} }
...@@ -68,24 +62,16 @@ void WebRtcLocalAudioRenderer::OnData(const media::AudioBus& audio_bus, ...@@ -68,24 +62,16 @@ void WebRtcLocalAudioRenderer::OnData(const media::AudioBus& audio_bus,
TRACE_EVENT0("audio", "WebRtcLocalAudioRenderer::CaptureData"); TRACE_EVENT0("audio", "WebRtcLocalAudioRenderer::CaptureData");
base::AutoLock auto_lock(thread_lock_); base::AutoLock auto_lock(thread_lock_);
if (!playing_ || !volume_ || !loopback_fifo_) if (!playing_ || !volume_ || !audio_shifter_)
return; return;
// Push captured audio to FIFO so it can be read by a local sink. scoped_ptr<media::AudioBus> audio_data(
if ((loopback_fifo_->frames() + audio_bus.frames()) <= media::AudioBus::Create(audio_bus.channels(), audio_bus.frames()));
loopback_fifo_->max_frames()) { audio_bus.CopyTo(audio_data.get());
// TODO(miu): Make sure the Render() method accounts for time shifting audio_shifter_->Push(audio_data.Pass(), estimated_capture_time);
// appropriately, per the comments for the usage of the const base::TimeTicks now = base::TimeTicks::Now();
// |estimated_capture_time| field found in media_stream_audio_sink.h. total_render_time_ += now - last_render_time_;
// http://crbug.com/437064 last_render_time_ = now;
loopback_fifo_->Push(&audio_bus);
const base::TimeTicks now = base::TimeTicks::Now();
total_render_time_ += now - last_render_time_;
last_render_time_ = now;
} else {
DVLOG(1) << "FIFO is full";
}
} }
void WebRtcLocalAudioRenderer::OnSetFormat( void WebRtcLocalAudioRenderer::OnSetFormat(
...@@ -152,7 +138,7 @@ void WebRtcLocalAudioRenderer::Stop() { ...@@ -152,7 +138,7 @@ void WebRtcLocalAudioRenderer::Stop() {
{ {
base::AutoLock auto_lock(thread_lock_); base::AutoLock auto_lock(thread_lock_);
playing_ = false; playing_ = false;
loopback_fifo_.reset(); audio_shifter_.reset();
} }
// Stop the output audio stream, i.e, stop asking for data to render. // Stop the output audio stream, i.e, stop asking for data to render.
...@@ -246,7 +232,7 @@ void WebRtcLocalAudioRenderer::MaybeStartSink() { ...@@ -246,7 +232,7 @@ void WebRtcLocalAudioRenderer::MaybeStartSink() {
{ {
// Clear up the old data in the FIFO. // Clear up the old data in the FIFO.
base::AutoLock auto_lock(thread_lock_); base::AutoLock auto_lock(thread_lock_);
loopback_fifo_->Clear(); audio_shifter_->Flush();
} }
if (!sink_params_.IsValid() || !playing_ || !volume_ || sink_started_) if (!sink_params_.IsValid() || !playing_ || !volume_ || sink_started_)
...@@ -296,19 +282,19 @@ void WebRtcLocalAudioRenderer::ReconfigureSink( ...@@ -296,19 +282,19 @@ void WebRtcLocalAudioRenderer::ReconfigureSink(
source_params_.effects() | implicit_ducking_effect); source_params_.effects() | implicit_ducking_effect);
{ {
// TODO(henrika): we could add a more dynamic solution here but I prefer // Note: The max buffer is fairly large, but will rarely be used.
// a fixed size combined with bad audio at overflow. The alternative is // Cast needs the buffer to hold at least one second of audio.
// that we start to build up latency and that can be more difficult to // The clock accuracy is set to 20ms because clock accuracy is
// detect. Tests have shown that the FIFO never contains more than 2 or 3 // ~15ms on windows.
// audio frames but I have selected a max size of ten buffers just media::AudioShifter* const new_shifter = new media::AudioShifter(
// in case since these tests were performed on a 16 core, 64GB Win 7 base::TimeDelta::FromSeconds(2),
// machine. We could also add some sort of error notifier in this area if base::TimeDelta::FromMilliseconds(20),
// the FIFO overflows. base::TimeDelta::FromSeconds(20),
media::AudioFifo* const new_fifo = new media::AudioFifo( source_params_.sample_rate(),
params.channels(), 10 * params.frames_per_buffer()); params.channels());
base::AutoLock auto_lock(thread_lock_); base::AutoLock auto_lock(thread_lock_);
loopback_fifo_.reset(new_fifo); audio_shifter_.reset(new_shifter);
} }
if (!sink_.get()) if (!sink_.get())
......
...@@ -21,7 +21,7 @@ ...@@ -21,7 +21,7 @@
namespace media { namespace media {
class AudioBus; class AudioBus;
class AudioFifo; class AudioShifter;
class AudioOutputDevice; class AudioOutputDevice;
class AudioParameters; class AudioParameters;
} }
...@@ -120,8 +120,8 @@ class CONTENT_EXPORT WebRtcLocalAudioRenderer ...@@ -120,8 +120,8 @@ class CONTENT_EXPORT WebRtcLocalAudioRenderer
// The sink (destination) for rendered audio. // The sink (destination) for rendered audio.
scoped_refptr<media::AudioOutputDevice> sink_; scoped_refptr<media::AudioOutputDevice> sink_;
// Contains copies of captured audio frames. // This does all the synchronization/resampling/smoothing.
scoped_ptr<media::AudioFifo> loopback_fifo_; scoped_ptr<media::AudioShifter> audio_shifter_;
// Stores last time a render callback was received. The time difference // Stores last time a render callback was received. The time difference
// between a new time stamp and this value can be used to derive the // between a new time stamp and this value can be used to derive the
...@@ -142,7 +142,7 @@ class CONTENT_EXPORT WebRtcLocalAudioRenderer ...@@ -142,7 +142,7 @@ class CONTENT_EXPORT WebRtcLocalAudioRenderer
// Set when playing, cleared when paused. // Set when playing, cleared when paused.
bool playing_; bool playing_;
// Protects |loopback_fifo_|, |playing_| and |sink_|. // Protects |audio_shifter_|, |playing_| and |sink_|.
mutable base::Lock thread_lock_; mutable base::Lock thread_lock_;
// The preferred buffer size provided via the ctor. // The preferred buffer size provided via the ctor.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment