diff options
author | acolwell@chromium.org <acolwell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-24 07:58:52 +0000 |
---|---|---|
committer | acolwell@chromium.org <acolwell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-05-24 07:58:52 +0000 |
commit | 34e4b51c367c86330f011d1803a1c9c0338d4f33 (patch) | |
tree | 7793ab46441669b6729e72952d7305acf1a62669 | |
parent | 4d954397c8ae3cb05535c402c237821c506bf465 (diff) | |
download | chromium_src-34e4b51c367c86330f011d1803a1c9c0338d4f33.zip chromium_src-34e4b51c367c86330f011d1803a1c9c0338d4f33.tar.gz chromium_src-34e4b51c367c86330f011d1803a1c9c0338d4f33.tar.bz2 |
Fix various MediaSource related crashes on Android.
This change fixes several issues that were causing crashes.
- MediaPlayerHostMsg_DemuxerReady_Params was not initializing the xxx_codec
member variables so audio-only or video-only content would randomly be
flagged as A/V content with a random codec ID.
- ChunkDemuxer::Stop() was not being called which could cause the delegate
to be accessed after it was destroyed.
- MediaSourceDelegate was signalling that it had metadata before the
ChunkDemuxer was even initialized. This caused the HTMLMediaElement to
signal that the load completed way too early and caused play() to be
called when the delegate wasn't ready.
BUG=233420
TEST=webkitmediasource-play.html LayoutTest reliably runs w/o crashing now.
Review URL: https://chromiumcodereview.appspot.com/15898002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@202019 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r-- | media/base/android/demuxer_stream_player_params.cc | 4 | ||||
-rw-r--r-- | media/base/android/media_source_player.cc | 9 | ||||
-rw-r--r-- | webkit/media/android/media_source_delegate.cc | 51 | ||||
-rw-r--r-- | webkit/media/android/media_source_delegate.h | 21 | ||||
-rw-r--r-- | webkit/media/android/webmediaplayer_android.cc | 2 | ||||
-rw-r--r-- | webkit/media/android/webmediaplayer_android.h | 6 |
6 files changed, 78 insertions, 15 deletions
diff --git a/media/base/android/demuxer_stream_player_params.cc b/media/base/android/demuxer_stream_player_params.cc index ab9fcaf..3ed1a8c 100644 --- a/media/base/android/demuxer_stream_player_params.cc +++ b/media/base/android/demuxer_stream_player_params.cc @@ -8,9 +8,11 @@ namespace media { MediaPlayerHostMsg_DemuxerReady_Params:: MediaPlayerHostMsg_DemuxerReady_Params() - : audio_channels(0), + : audio_codec(kUnknownAudioCodec), + audio_channels(0), audio_sampling_rate(0), is_audio_encrypted(false), + video_codec(kUnknownVideoCodec), is_video_encrypted(false), duration_ms(0) {} diff --git a/media/base/android/media_source_player.cc b/media/base/android/media_source_player.cc index b29d182..f6cbd48 100644 --- a/media/base/android/media_source_player.cc +++ b/media/base/android/media_source_player.cc @@ -208,7 +208,6 @@ MediaSourcePlayer::MediaSourcePlayer( waiting_for_audio_data_(false), waiting_for_video_data_(false), weak_this_(this) { - OnMediaMetadataChanged(duration_, width_, height_, false); } MediaSourcePlayer::~MediaSourcePlayer() { @@ -221,9 +220,11 @@ void MediaSourcePlayer::SetVideoSurface(jobject surface) { return; } - video_decoder_job_.reset(new VideoDecoderJob( - base::MessageLoopProxy::current(), video_codec_, - gfx::Size(width_, height_), surface)); + if (HasVideo()) { + video_decoder_job_.reset(new VideoDecoderJob( + base::MessageLoopProxy::current(), video_codec_, + gfx::Size(width_, height_), surface)); + } if (pending_play_) StartInternal(); diff --git a/webkit/media/android/media_source_delegate.cc b/webkit/media/android/media_source_delegate.cc index bf15894..66e7563 100644 --- a/webkit/media/android/media_source_delegate.cc +++ b/webkit/media/android/media_source_delegate.cc @@ -88,7 +88,26 @@ MediaSourceDelegate::MediaSourceDelegate( } } -MediaSourceDelegate::~MediaSourceDelegate() {} +MediaSourceDelegate::~MediaSourceDelegate() { + DVLOG(1) << "MediaSourceDelegate::~MediaSourceDelegate() : " << player_id_; + DCHECK(!chunk_demuxer_); +} + +void MediaSourceDelegate::Destroy() { + DVLOG(1) << "MediaSourceDelegate::Destroy() : " << player_id_; + if (!chunk_demuxer_) { + delete this; + return; + } + + update_network_state_cb_.Reset(); + media_source_.reset(); + client_ = NULL; + proxy_ = NULL; + + chunk_demuxer_->Stop( + BIND_TO_RENDER_LOOP(&MediaSourceDelegate::OnDemuxerStopDone)); +} void MediaSourceDelegate::Initialize( WebKit::WebMediaSource* media_source, @@ -199,6 +218,8 @@ WebMediaPlayer::MediaKeyException MediaSourceDelegate::CancelKeyRequest( } void MediaSourceDelegate::Seek(base::TimeDelta time) { + DVLOG(1) << "MediaSourceDelegate::Seek(" << time.InSecondsF() << ") : " + << player_id_; seeking_ = true; DCHECK(chunk_demuxer_); if (!chunk_demuxer_) @@ -227,6 +248,8 @@ void MediaSourceDelegate::SetDuration(base::TimeDelta duration) { void MediaSourceDelegate::OnReadFromDemuxer(media::DemuxerStream::Type type, bool seek_done) { + DVLOG(1) << "MediaSourceDelegate::OnReadFromDemuxer(" << type + << ", " << seek_done << ") : " << player_id_; if (seeking_ && !seek_done) return; // Drop the request during seeking. seeking_ = false; @@ -255,6 +278,7 @@ void MediaSourceDelegate::OnBufferReady( size_t index, DemuxerStream::Status status, const scoped_refptr<media::DecoderBuffer>& buffer) { + DVLOG(1) << "MediaSourceDelegate::OnBufferReady() : " << player_id_; DCHECK(status == DemuxerStream::kAborted || index < params->access_units.size()); bool is_audio = stream->type() == DemuxerStream::AUDIO; @@ -342,6 +366,8 @@ void MediaSourceDelegate::OnBufferReady( void MediaSourceDelegate::OnDemuxerError( media::PipelineStatus status) { + DVLOG(1) << "MediaSourceDelegate::OnDemuxerError(" << status << ") : " + << player_id_; if (status != media::PIPELINE_OK) { DCHECK(status == media::DEMUXER_ERROR_COULD_NOT_OPEN || status == media::DEMUXER_ERROR_COULD_NOT_PARSE || @@ -354,6 +380,8 @@ void MediaSourceDelegate::OnDemuxerError( void MediaSourceDelegate::OnDemuxerInitDone( media::PipelineStatus status) { + DVLOG(1) << "MediaSourceDelegate::OnDemuxerInitDone(" << status << ") : " + << player_id_; if (status != media::PIPELINE_OK) { OnDemuxerError(status); return; @@ -361,6 +389,12 @@ void MediaSourceDelegate::OnDemuxerInitDone( NotifyDemuxerReady(""); } +void MediaSourceDelegate::OnDemuxerStopDone() { + DVLOG(1) << "MediaSourceDelegate::OnDemuxerStopDone() : " << player_id_; + chunk_demuxer_.reset(); + delete this; +} + void MediaSourceDelegate::NotifyDemuxerReady(const std::string& key_system) { MediaPlayerHostMsg_DemuxerReady_Params params; DemuxerStream* audio_stream = chunk_demuxer_->GetStream(DemuxerStream::AUDIO); @@ -399,6 +433,9 @@ void MediaSourceDelegate::NotifyDemuxerReady(const std::string& key_system) { } void MediaSourceDelegate::OnDemuxerOpened() { + if (!media_source_) + return; + media_source_->open(new WebMediaSourceClientImpl( chunk_demuxer_.get(), base::Bind(&LogMediaSourceError, media_log_))); } @@ -407,6 +444,9 @@ void MediaSourceDelegate::OnKeyError(const std::string& key_system, const std::string& session_id, media::Decryptor::KeyError error_code, int system_code) { + if (!client_) + return; + client_->keyError( WebString::fromUTF8(key_system), WebString::fromUTF8(session_id), @@ -422,6 +462,9 @@ void MediaSourceDelegate::OnKeyMessage(const std::string& key_system, DLOG_IF(WARNING, !default_url.empty() && !default_url_gurl.is_valid()) << "Invalid URL in default_url: " << default_url; + if (!client_) + return; + client_->keyMessage(WebString::fromUTF8(key_system), WebString::fromUTF8(session_id), reinterpret_cast<const uint8*>(message.data()), @@ -431,7 +474,11 @@ void MediaSourceDelegate::OnKeyMessage(const std::string& key_system, void MediaSourceDelegate::OnKeyAdded(const std::string& key_system, const std::string& session_id) { + if (!client_) + return; + NotifyDemuxerReady(key_system); + client_->keyAdded(WebString::fromUTF8(key_system), WebString::fromUTF8(session_id)); } @@ -442,7 +489,7 @@ void MediaSourceDelegate::OnNeedKey(const std::string& key_system, scoped_ptr<uint8[]> init_data, int init_data_size) { // Do not fire NeedKey event if encrypted media is not enabled. - if (!decryptor_) + if (!client_ || !decryptor_) return; CHECK(init_data_size >= 0); diff --git a/webkit/media/android/media_source_delegate.h b/webkit/media/android/media_source_delegate.h index f2cf340..6ce16b1 100644 --- a/webkit/media/android/media_source_delegate.h +++ b/webkit/media/android/media_source_delegate.h @@ -40,13 +40,20 @@ class MediaSourceDelegate : public media::DemuxerHost { typedef base::Callback<void(WebKit::WebMediaPlayer::NetworkState)> UpdateNetworkStateCB; + // Helper class used by scoped_ptr to destroy an instance of + // MediaSourceDelegate. + class Destroyer { + public: + inline void operator()(void* media_source_delegate) const { + static_cast<MediaSourceDelegate*>(media_source_delegate)->Destroy(); + } + }; + MediaSourceDelegate(WebKit::WebFrame* frame, WebKit::WebMediaPlayerClient* client, WebMediaPlayerProxyAndroid* proxy, int player_id, media::MediaLog* media_log); - virtual ~MediaSourceDelegate(); - // Initialize the MediaSourceDelegate. |media_source| will be owned by // this object after this call. void Initialize(WebKit::WebMediaSource* media_source, @@ -79,7 +86,13 @@ class MediaSourceDelegate : public media::DemuxerHost { // If it's the first request after the seek, |seek_done| will be true. void OnReadFromDemuxer(media::DemuxerStream::Type type, bool seek_done); + // Called by the Destroyer to destroy an instance of this object. + void Destroy(); + private: + // This is private to enforce use of the Destroyer. + virtual ~MediaSourceDelegate(); + // Methods inherited from DemuxerHost. virtual void SetTotalBytes(int64 total_bytes) OVERRIDE; virtual void AddBufferedByteRange(int64 start, int64 end) OVERRIDE; @@ -90,6 +103,7 @@ class MediaSourceDelegate : public media::DemuxerHost { // Callbacks for ChunkDemuxer & Decryptor. void OnDemuxerInitDone(media::PipelineStatus status); + void OnDemuxerStopDone(); void OnDemuxerOpened(); void OnKeyAdded(const std::string& key_system, const std::string& session_id); void OnKeyError(const std::string& key_system, @@ -127,7 +141,7 @@ class MediaSourceDelegate : public media::DemuxerHost { base::WeakPtrFactory<MediaSourceDelegate> weak_this_; - WebKit::WebMediaPlayerClient* const client_; + WebKit::WebMediaPlayerClient* client_; WebMediaPlayerProxyAndroid* proxy_; int player_id_; @@ -163,4 +177,3 @@ class MediaSourceDelegate : public media::DemuxerHost { } // namespace webkit_media #endif // WEBKIT_MEDIA_ANDROID_MEDIA_SOURCE_DELEGATE_H_ - diff --git a/webkit/media/android/webmediaplayer_android.cc b/webkit/media/android/webmediaplayer_android.cc index 973c5f2..acfeb5d 100644 --- a/webkit/media/android/webmediaplayer_android.cc +++ b/webkit/media/android/webmediaplayer_android.cc @@ -21,7 +21,6 @@ #include "third_party/WebKit/Source/WebKit/chromium/public/WebMediaPlayerClient.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebMediaSource.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebView.h" -#include "webkit/media/android/media_source_delegate.h" #include "webkit/media/android/webmediaplayer_manager_android.h" #include "webkit/media/android/webmediaplayer_proxy_android.h" #include "webkit/media/webmediaplayer_util.h" @@ -552,6 +551,7 @@ void WebMediaPlayerAndroid::Detach() { stream_id_ = 0; } + media_source_delegate_.reset(); current_frame_ = NULL; manager_ = NULL; proxy_ = NULL; diff --git a/webkit/media/android/webmediaplayer_android.h b/webkit/media/android/webmediaplayer_android.h index c492910..f344086 100644 --- a/webkit/media/android/webmediaplayer_android.h +++ b/webkit/media/android/webmediaplayer_android.h @@ -19,6 +19,7 @@ #include "third_party/WebKit/Source/Platform/chromium/public/WebURL.h" #include "third_party/WebKit/Source/WebKit/chromium/public/WebMediaPlayer.h" #include "ui/gfx/rect_f.h" +#include "webkit/media/android/media_source_delegate.h" #include "webkit/media/android/stream_texture_factory_android.h" namespace media { @@ -34,8 +35,6 @@ class WebLayerImpl; } namespace webkit_media { - -class MediaSourceDelegate; class WebMediaPlayerManagerAndroid; class WebMediaPlayerProxyAndroid; @@ -304,7 +303,8 @@ class WebMediaPlayerAndroid gfx::RectF last_computed_rect_; #endif - scoped_ptr<MediaSourceDelegate> media_source_delegate_; + scoped_ptr<MediaSourceDelegate, + MediaSourceDelegate::Destroyer> media_source_delegate_; // Proxy object that delegates method calls on Render Thread. // This object is created on the Render Thread and is only called in the |