From 0a40d9db89298db3edb7cbb49488db1100e0cdbb Mon Sep 17 00:00:00 2001 From: sigbjornf Date: Wed, 9 Mar 2016 00:50:16 -0800 Subject: Revert of MediaStream audio object graph untangling and clean-ups. (patchset #10 id:200001 of https://codereview.chromium.org/1721273002/ ) Reason for revert: Broke a number of mediastream/ tests, e.g., https://build.chromium.org/p/chromium.webkit/builders/WebKit%20Linux%20Trusty/builds/9786 Original issue's description: > MediaStream audio object graph untangling and clean-ups. > > 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=peter@chromium.org > > Committed: https://crrev.com/26bfd80549511a7e05f23c9941c41ced104ddf28 > Cr-Commit-Position: refs/heads/master@{#380065} TBR=jochen@chromium.org,finnur@chromium.org,mcasas@chromium.org,olka@chromium.org,peter@chromium.org,tommi@chromium.org,miu@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=577881, 577874 Review URL: https://codereview.chromium.org/1780653002 Cr-Commit-Position: refs/heads/master@{#380103} --- 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, 109 insertions(+), 85 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 5baaa54..9ec76b7 100644 --- a/content/public/renderer/media_stream_api.cc +++ b/content/public/renderer/media_stream_api.cc @@ -6,9 +6,8 @@ #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" @@ -23,92 +22,112 @@ namespace content { -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"; +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"; return false; } - - blink::WebMediaStreamSource web_media_stream_source; - MediaStreamVideoSource* const media_stream_source = + const blink::WebString track_id = MakeTrackId(); + blink::WebMediaStreamSource webkit_source; + scoped_ptr media_stream_source( new MediaStreamVideoCapturerSource( - 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); + 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()); blink::WebMediaConstraints constraints; constraints.initialize(); - web_media_stream->addTrack(MediaStreamVideoTrack::CreateVideoTrack( - media_stream_source, constraints, + web_stream->addTrack(MediaStreamVideoTrack::CreateVideoTrack( + media_stream_source.release(), constraints, MediaStreamVideoSource::ConstraintsCallback(), true)); return true; } bool AddAudioTrackToMediaStream( - scoped_refptr audio_source, - int sample_rate, - media::ChannelLayout channel_layout, - int frames_per_buffer, + const scoped_refptr& source, + const media::AudioParameters& params, bool is_remote, bool is_readonly, - blink::WebMediaStream* web_media_stream) { - DCHECK(audio_source.get()); - if (!web_media_stream || web_media_stream->isNull()) { - DLOG(ERROR) << "WebMediaStream is null"; - return false; - } + 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); +} - 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."; +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"; return false; } - 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); + media::AudioParameters params( + media::AudioParameters::AUDIO_PCM_LINEAR, media::CHANNEL_LAYOUT_STEREO, + 48000, /* sample rate */ + 16, /* bits per sample */ + 480); /* frames per buffer */ - MediaStreamAudioSource* media_stream_source(new MediaStreamAudioSource( - -1, StreamDeviceInfo(), MediaStreamSource::SourceStoppedCallback(), - RenderThreadImpl::current()->GetPeerConnectionDependencyFactory())); + blink::WebMediaStreamSource webkit_source; + const blink::WebString track_id = MakeTrackId(); + webkit_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())); blink::WebMediaConstraints constraints; constraints.initialize(); - { - // 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. + scoped_refptr capturer( + WebRtcAudioCapturer::CreateCapturer(-1, StreamDeviceInfo(), constraints, + nullptr, audio_source)); + capturer->SetCapturerSource(source, params); + audio_source->SetAudioCapturer(capturer); + webkit_source.setExtraData(audio_source); - blink::WebMediaStreamTrack web_media_stream_track; - web_media_stream_track.initialize(web_media_stream_source); - RenderThreadImpl::current() - ->GetPeerConnectionDependencyFactory() - ->CreateLocalAudioTrack(web_media_stream_track); + blink::WebMediaStreamTrack web_media_audio_track; + web_media_audio_track.initialize(webkit_source); + RenderThreadImpl::current()->GetPeerConnectionDependencyFactory()-> + CreateLocalAudioTrack(web_media_audio_track); - web_media_stream->addTrack(web_media_stream_track); + web_stream->addTrack(web_media_audio_track); return true; } diff --git a/content/public/renderer/media_stream_api.h b/content/public/renderer/media_stream_api.h index b87c93d..dfb2aaf 100644 --- a/content/public/renderer/media_stream_api.h +++ b/content/public/renderer/media_stream_api.h @@ -7,39 +7,47 @@ #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, connected to the source and is plugged into -// the WebMediaStream (|web_media_stream|). +// MediaStreamTrack pair is created, holding the previous MediaStreamSource, and +// is plugged into the stream identified by |media_stream_url| (or passed as +// |web_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 video_source, + scoped_ptr source, + bool is_remote, + bool is_readonly, + const std::string& media_stream_url); +CONTENT_EXPORT bool AddVideoTrackToMediaStream( + scoped_ptr source, bool is_remote, bool is_readonly, - blink::WebMediaStream* web_media_stream); + blink::WebMediaStream* web_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( - scoped_refptr audio_source, - int sample_rate, - media::ChannelLayout channel_layout, - int frames_per_buffer, + 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, bool is_remote, bool is_readonly, - blink::WebMediaStream* web_media_stream); + blink::WebMediaStream* web_stream); } // namespace content diff --git a/content/public/renderer/media_stream_audio_sink.cc b/content/public/renderer/media_stream_audio_sink.cc index 875a635..ad075d1 100644 --- a/content/public/renderer/media_stream_audio_sink.cc +++ b/content/public/renderer/media_stream_audio_sink.cc @@ -15,23 +15,20 @@ void MediaStreamAudioSink::AddToAudioTrack( MediaStreamAudioSink* sink, const blink::WebMediaStreamTrack& track) { DCHECK(track.source().getType() == blink::WebMediaStreamSource::TypeAudio); - MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::From(track); - DCHECK(native_track); + MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::GetTrack(track); native_track->AddSink(sink); } void MediaStreamAudioSink::RemoveFromAudioTrack( MediaStreamAudioSink* sink, const blink::WebMediaStreamTrack& track) { - MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::From(track); - DCHECK(native_track); + MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::GetTrack(track); native_track->RemoveSink(sink); } media::AudioParameters MediaStreamAudioSink::GetFormatFromAudioTrack( const blink::WebMediaStreamTrack& track) { - MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::From(track); - DCHECK(native_track); + MediaStreamAudioTrack* native_track = MediaStreamAudioTrack::GetTrack(track); return native_track->GetOutputFormat(); } -- cgit v1.1