From cfbc8cb058dcc6cf88f1c4d70c8eb76652c39123 Mon Sep 17 00:00:00 2001 From: miu Date: Wed, 9 Mar 2016 13:09:13 -0800 Subject: RELAND: MediaStream audio object graph untangling and clean-ups. This is a re-land of https://codereview.chromium.org/1721273002 with the signaling thread check in WebRtcLocalAudioTrackAdapter::Create() reverted back to the way it was. This is needed because the layout tests do not run in a full browser environment where the WebRTC signaling thread is present. ------------------ORIGINAL DESCRIPTION FOLLOWS------------------ This change consists of a number of "clean-up" changes that are being done to make the soon-upcoming refactoring of these classes go much more smoothly. These are: 1. Public content MediaStreamApi functions cleaned up. Removed "duplicated" functions that don't really do the same thing. Removed hard-coded audio parameters from AddAudioTrackToMediaStream(). 2. Eliminated ref-counting of WebRtcAudioCapturer and WebAudioCaptureSource. Removed unnecessary references to these from WebRtcLocalAudioTrack. Not only did this improve encapsulation of some code, but it also allowed for the removal of several dozen lines of "dead weight" testing set-upcode throughout the directory. 3. Renamed MediaStreamAudioTrack::GetTrack() method to From(), to be consistent with how this pattern is used in other parts of libcontent, and added a MediaStreamAudioSource::From(). 4. Moved audio level calculations out of WebRtcLocalAudioTrack and into WebRtcAudioCapturer. This way, when multiple tracks are present, the calculation is only being done once on the same audio. 5. Eliminated call to WebRtcCapturer::Stop() from WebRtcAudioDeviceImpl::Terminate(), which are each supposed to run on different threads. From testing, DCHECKs should have been firing, but weren't, so the Terminate() method seems to be dead code. 6. Miscellaneous other "compaction" and comment updates. BUG=577881, 577874 TBR=jochen@chromium.org,finnur@chromium.org,o1ka@chromium.org,tommi@chromium.org,peter@chromium.org,sof@chromium.org Review URL: https://codereview.chromium.org/1780743002 Cr-Commit-Position: refs/heads/master@{#380214} --- content/public/renderer/media_stream_api.cc | 149 +++++++++------------ content/public/renderer/media_stream_api.h | 36 ++--- content/public/renderer/media_stream_audio_sink.cc | 9 +- 3 files changed, 85 insertions(+), 109 deletions(-) (limited to 'content/public/renderer') diff --git a/content/public/renderer/media_stream_api.cc b/content/public/renderer/media_stream_api.cc index 9ec76b7..5baaa54 100644 --- a/content/public/renderer/media_stream_api.cc +++ b/content/public/renderer/media_stream_api.cc @@ -6,8 +6,9 @@ #include -#include "base/base64.h" #include "base/callback.h" +#include "base/guid.h" +#include "base/memory/scoped_ptr.h" #include "base/rand_util.h" #include "base/strings/utf_string_conversions.h" #include "content/renderer/media/media_stream_audio_source.h" @@ -22,112 +23,92 @@ namespace content { -namespace { - -blink::WebString MakeTrackId() { - std::string track_id; - base::Base64Encode(base::RandBytesAsString(64), &track_id); - return base::UTF8ToUTF16(track_id); -} - -} // namespace - -bool AddVideoTrackToMediaStream(scoped_ptr source, - bool is_remote, - bool is_readonly, - const std::string& media_stream_url) { - blink::WebMediaStream web_stream = - blink::WebMediaStreamRegistry::lookupMediaStreamDescriptor( - GURL(media_stream_url)); - return AddVideoTrackToMediaStream(std::move(source), is_remote, is_readonly, - &web_stream); -} - -bool AddVideoTrackToMediaStream(scoped_ptr source, - bool is_remote, - bool is_readonly, - blink::WebMediaStream* web_stream) { - if (web_stream->isNull()) { - DLOG(ERROR) << "Stream not found"; +bool AddVideoTrackToMediaStream( + scoped_ptr video_source, + bool is_remote, + bool is_readonly, + blink::WebMediaStream* web_media_stream) { + DCHECK(video_source.get()); + if (!web_media_stream || web_media_stream->isNull()) { + DLOG(ERROR) << "WebMediaStream is null"; return false; } - const blink::WebString track_id = MakeTrackId(); - blink::WebMediaStreamSource webkit_source; - scoped_ptr media_stream_source( + + blink::WebMediaStreamSource web_media_stream_source; + MediaStreamVideoSource* const media_stream_source = new MediaStreamVideoCapturerSource( - MediaStreamSource::SourceStoppedCallback(), std::move(source))); - webkit_source.initialize(track_id, blink::WebMediaStreamSource::TypeVideo, - track_id, is_remote, is_readonly); - webkit_source.setExtraData(media_stream_source.get()); + MediaStreamSource::SourceStoppedCallback(), std::move(video_source)); + const blink::WebString track_id = + blink::WebString::fromUTF8(base::GenerateGUID()); + web_media_stream_source.initialize(track_id, + blink::WebMediaStreamSource::TypeVideo, + track_id, is_remote, is_readonly); + // Takes ownership of |media_stream_source|. + web_media_stream_source.setExtraData(media_stream_source); blink::WebMediaConstraints constraints; constraints.initialize(); - web_stream->addTrack(MediaStreamVideoTrack::CreateVideoTrack( - media_stream_source.release(), constraints, + web_media_stream->addTrack(MediaStreamVideoTrack::CreateVideoTrack( + media_stream_source, constraints, MediaStreamVideoSource::ConstraintsCallback(), true)); return true; } bool AddAudioTrackToMediaStream( - const scoped_refptr& source, - const media::AudioParameters& params, + scoped_refptr audio_source, + int sample_rate, + media::ChannelLayout channel_layout, + int frames_per_buffer, bool is_remote, bool is_readonly, - const std::string& media_stream_url) { - DCHECK(params.IsValid()) << params.AsHumanReadableString(); - blink::WebMediaStream web_stream = - blink::WebMediaStreamRegistry::lookupMediaStreamDescriptor( - GURL(media_stream_url)); - return AddAudioTrackToMediaStream(source, - is_remote, is_readonly, &web_stream); -} - -bool AddAudioTrackToMediaStream( - const scoped_refptr& source, - bool is_remote, - bool is_readonly, - blink::WebMediaStream* web_stream) { - if (web_stream->isNull()) { - DLOG(ERROR) << "Stream not found"; + blink::WebMediaStream* web_media_stream) { + DCHECK(audio_source.get()); + if (!web_media_stream || web_media_stream->isNull()) { + DLOG(ERROR) << "WebMediaStream is null"; return false; } - media::AudioParameters params( - media::AudioParameters::AUDIO_PCM_LINEAR, media::CHANNEL_LAYOUT_STEREO, - 48000, /* sample rate */ - 16, /* bits per sample */ - 480); /* frames per buffer */ + const media::AudioParameters params( + media::AudioParameters::AUDIO_PCM_LOW_LATENCY, channel_layout, + sample_rate, sizeof(int16_t) * 8, frames_per_buffer); + if (!params.IsValid()) { + DLOG(ERROR) << "Invalid audio parameters."; + return false; + } - blink::WebMediaStreamSource webkit_source; - const blink::WebString track_id = MakeTrackId(); - webkit_source.initialize(track_id, - blink::WebMediaStreamSource::TypeAudio, - track_id, - is_remote, - is_readonly); + blink::WebMediaStreamSource web_media_stream_source; + const blink::WebString track_id = + blink::WebString::fromUTF8(base::GenerateGUID()); + web_media_stream_source.initialize(track_id, + blink::WebMediaStreamSource::TypeAudio, + track_id, is_remote, is_readonly); - MediaStreamAudioSource* audio_source( - new MediaStreamAudioSource( - -1, - StreamDeviceInfo(), - MediaStreamSource::SourceStoppedCallback(), - RenderThreadImpl::current()->GetPeerConnectionDependencyFactory())); + MediaStreamAudioSource* media_stream_source(new MediaStreamAudioSource( + -1, StreamDeviceInfo(), MediaStreamSource::SourceStoppedCallback(), + RenderThreadImpl::current()->GetPeerConnectionDependencyFactory())); blink::WebMediaConstraints constraints; constraints.initialize(); - scoped_refptr capturer( - WebRtcAudioCapturer::CreateCapturer(-1, StreamDeviceInfo(), constraints, - nullptr, audio_source)); - capturer->SetCapturerSource(source, params); - audio_source->SetAudioCapturer(capturer); - webkit_source.setExtraData(audio_source); + { + // TODO(miu): In an upcoming change, a source purposed for passing audio + // directly (i.e., without modification) will replace this "hacky" use of + // WebRtcAudioCapturer. http://crbug.com/577881 + scoped_ptr capturer( + WebRtcAudioCapturer::CreateCapturer(-1, StreamDeviceInfo(), constraints, + nullptr, media_stream_source)); + capturer->SetCapturerSource(std::move(audio_source), params); + media_stream_source->SetAudioCapturer(std::move(capturer)); + } + web_media_stream_source.setExtraData( + media_stream_source); // Takes ownership. - blink::WebMediaStreamTrack web_media_audio_track; - web_media_audio_track.initialize(webkit_source); - RenderThreadImpl::current()->GetPeerConnectionDependencyFactory()-> - CreateLocalAudioTrack(web_media_audio_track); + blink::WebMediaStreamTrack web_media_stream_track; + web_media_stream_track.initialize(web_media_stream_source); + RenderThreadImpl::current() + ->GetPeerConnectionDependencyFactory() + ->CreateLocalAudioTrack(web_media_stream_track); - web_stream->addTrack(web_media_audio_track); + web_media_stream->addTrack(web_media_stream_track); return true; } diff --git a/content/public/renderer/media_stream_api.h b/content/public/renderer/media_stream_api.h index dfb2aaf..b87c93d 100644 --- a/content/public/renderer/media_stream_api.h +++ b/content/public/renderer/media_stream_api.h @@ -7,47 +7,39 @@ #include "content/common/content_export.h" #include "media/base/audio_capturer_source.h" +#include "media/base/channel_layout.h" #include "media/base/video_capturer_source.h" namespace blink { class WebMediaStream; } -namespace media { -class AudioParameters; -} - namespace content { // These methods create a WebMediaStreamSource + MediaStreamSource pair with the // provided audio or video capturer source. A new WebMediaStreamTrack + -// MediaStreamTrack pair is created, holding the previous MediaStreamSource, and -// is plugged into the stream identified by |media_stream_url| (or passed as -// |web_stream|). +// MediaStreamTrack pair is created, connected to the source and is plugged into +// the WebMediaStream (|web_media_stream|). // |is_remote| should be true if the source of the data is not a local device. // |is_readonly| should be true if the format of the data cannot be changed by // MediaTrackConstraints. CONTENT_EXPORT bool AddVideoTrackToMediaStream( - scoped_ptr source, - bool is_remote, - bool is_readonly, - const std::string& media_stream_url); -CONTENT_EXPORT bool AddVideoTrackToMediaStream( - scoped_ptr source, + scoped_ptr video_source, bool is_remote, bool is_readonly, - blink::WebMediaStream* web_stream); + blink::WebMediaStream* web_media_stream); +// |sample_rate|, |channel_layout|, and |frames_per_buffer| specify the audio +// parameters of the track. Generally, these should match the |audio_source| so +// that it does not have to perform unnecessary sample rate conversion or +// channel mixing. CONTENT_EXPORT bool AddAudioTrackToMediaStream( - const scoped_refptr& source, - const media::AudioParameters& params, - bool is_remote, - bool is_readonly, - const std::string& media_stream_url); -CONTENT_EXPORT bool AddAudioTrackToMediaStream( - const scoped_refptr& source, + scoped_refptr audio_source, + int sample_rate, + media::ChannelLayout channel_layout, + int frames_per_buffer, bool is_remote, bool is_readonly, - blink::WebMediaStream* web_stream); + blink::WebMediaStream* web_media_stream); } // namespace content diff --git a/content/public/renderer/media_stream_audio_sink.cc b/content/public/renderer/media_stream_audio_sink.cc index ad075d1..875a635 100644 --- a/content/public/renderer/media_stream_audio_sink.cc +++ b/content/public/renderer/media_stream_audio_sink.cc @@ -15,20 +15,23 @@ void MediaStreamAudioSink::AddToAudioTrack( MediaStreamAudioSink* sink, const blink::WebMediaStreamTrack& track) { DCHECK(track.source().getType() == blink::WebMediaStreamSource::TypeAudio); - MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::GetTrack(track); + MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::From(track); + DCHECK(native_track); native_track->AddSink(sink); } void MediaStreamAudioSink::RemoveFromAudioTrack( MediaStreamAudioSink* sink, const blink::WebMediaStreamTrack& track) { - MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::GetTrack(track); + MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::From(track); + DCHECK(native_track); native_track->RemoveSink(sink); } media::AudioParameters MediaStreamAudioSink::GetFormatFromAudioTrack( const blink::WebMediaStreamTrack& track) { - MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::GetTrack(track); + MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::From(track); + DCHECK(native_track); return native_track->GetOutputFormat(); } -- cgit v1.1