summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authoracolwell@chromium.org <acolwell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-05-24 07:58:52 +0000
committeracolwell@chromium.org <acolwell@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-05-24 07:58:52 +0000
commit34e4b51c367c86330f011d1803a1c9c0338d4f33 (patch)
tree7793ab46441669b6729e72952d7305acf1a62669
parent4d954397c8ae3cb05535c402c237821c506bf465 (diff)
downloadchromium_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.cc4
-rw-r--r--media/base/android/media_source_player.cc9
-rw-r--r--webkit/media/android/media_source_delegate.cc51
-rw-r--r--webkit/media/android/media_source_delegate.h21
-rw-r--r--webkit/media/android/webmediaplayer_android.cc2
-rw-r--r--webkit/media/android/webmediaplayer_android.h6
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