Commit cdfc6f25 authored by Dale Curtis's avatar Dale Curtis Committed by Commit Bot

Query WebMediaPlayer when checking for presence of video frame.

ReadyState has never been suficient to test whether a video frame is
actually present. Instead we need to know when the frame has actually
been delivered to the compositor.

This patch updates HTMLVideoElement to query a new WebMediaPlayer method
which exposes the compositor's knowledge of the video frame.

Note: This patch also fixes some spurious logs and HTMLVideoElement tests
that were silently doing nothing.

BUG=974012,974190
TEST=new unittest.
TBR=eae

Change-Id: I1df54491c1fd7c2246ce0f60bc6e794b8b84476a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1659490
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: default avatarMounir Lamouri <mlamouri@chromium.org>
Cr-Commit-Position: refs/heads/master@{#672682}
parent 0447cd04
...@@ -828,6 +828,10 @@ uint64_t WebMediaPlayerMS::VideoDecodedByteCount() const { ...@@ -828,6 +828,10 @@ uint64_t WebMediaPlayerMS::VideoDecodedByteCount() const {
return 0; return 0;
} }
bool WebMediaPlayerMS::HasAvailableVideoFrame() const {
return has_first_frame_;
}
void WebMediaPlayerMS::OnFrameHidden() { void WebMediaPlayerMS::OnFrameHidden() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
// This method is called when the RenderFrame is sent to background or // This method is called when the RenderFrame is sent to background or
...@@ -1072,6 +1076,7 @@ void WebMediaPlayerMS::OnFirstFrameReceived(media::VideoRotation video_rotation, ...@@ -1072,6 +1076,7 @@ void WebMediaPlayerMS::OnFirstFrameReceived(media::VideoRotation video_rotation,
DVLOG(1) << __func__; DVLOG(1) << __func__;
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_); DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
has_first_frame_ = true;
OnRotationChanged(video_rotation); OnRotationChanged(video_rotation);
OnOpacityChanged(is_opaque); OnOpacityChanged(is_opaque);
......
...@@ -167,6 +167,8 @@ class CONTENT_EXPORT WebMediaPlayerMS ...@@ -167,6 +167,8 @@ class CONTENT_EXPORT WebMediaPlayerMS
uint64_t AudioDecodedByteCount() const override; uint64_t AudioDecodedByteCount() const override;
uint64_t VideoDecodedByteCount() const override; uint64_t VideoDecodedByteCount() const override;
bool HasAvailableVideoFrame() const override;
// WebMediaPlayerDelegate::Observer implementation. // WebMediaPlayerDelegate::Observer implementation.
void OnFrameHidden() override; void OnFrameHidden() override;
void OnFrameClosed() override; void OnFrameClosed() override;
...@@ -355,6 +357,8 @@ class CONTENT_EXPORT WebMediaPlayerMS ...@@ -355,6 +357,8 @@ class CONTENT_EXPORT WebMediaPlayerMS
// Whether the video is known to be opaque or not. // Whether the video is known to be opaque or not.
bool opaque_ = true; bool opaque_ = true;
bool has_first_frame_ = false;
base::WeakPtr<WebMediaPlayerMS> weak_this_; base::WeakPtr<WebMediaPlayerMS> weak_this_;
base::WeakPtrFactory<WebMediaPlayerMS> weak_factory_; base::WeakPtrFactory<WebMediaPlayerMS> weak_factory_;
......
...@@ -1266,6 +1266,10 @@ uint64_t WebMediaPlayerImpl::VideoDecodedByteCount() const { ...@@ -1266,6 +1266,10 @@ uint64_t WebMediaPlayerImpl::VideoDecodedByteCount() const {
return GetPipelineStatistics().video_bytes_decoded; return GetPipelineStatistics().video_bytes_decoded;
} }
bool WebMediaPlayerImpl::HasAvailableVideoFrame() const {
return has_first_frame_;
}
bool WebMediaPlayerImpl::CopyVideoTextureToPlatformTexture( bool WebMediaPlayerImpl::CopyVideoTextureToPlatformTexture(
gpu::gles2::GLES2Interface* gl, gpu::gles2::GLES2Interface* gl,
unsigned int target, unsigned int target,
...@@ -3474,6 +3478,10 @@ void WebMediaPlayerImpl::OnFirstFrame(base::TimeTicks frame_time) { ...@@ -3474,6 +3478,10 @@ void WebMediaPlayerImpl::OnFirstFrame(base::TimeTicks frame_time) {
const base::TimeDelta elapsed = frame_time - load_start_time_; const base::TimeDelta elapsed = frame_time - load_start_time_;
media_metrics_provider_->SetTimeToFirstFrame(elapsed); media_metrics_provider_->SetTimeToFirstFrame(elapsed);
RecordTimingUMA("Media.TimeToFirstFrame", elapsed); RecordTimingUMA("Media.TimeToFirstFrame", elapsed);
// Needed to signal HTMLVideoElement that it should remove the poster image.
if (client_ && did_lazy_load_ && has_poster_)
client_->Repaint();
} }
void WebMediaPlayerImpl::RecordTimingUMA(const std::string& key, void WebMediaPlayerImpl::RecordTimingUMA(const std::string& key,
......
...@@ -182,6 +182,8 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl ...@@ -182,6 +182,8 @@ class MEDIA_BLINK_EXPORT WebMediaPlayerImpl
uint64_t AudioDecodedByteCount() const override; uint64_t AudioDecodedByteCount() const override;
uint64_t VideoDecodedByteCount() const override; uint64_t VideoDecodedByteCount() const override;
bool HasAvailableVideoFrame() const override;
bool CopyVideoTextureToPlatformTexture( bool CopyVideoTextureToPlatformTexture(
gpu::gles2::GLES2Interface* gl, gpu::gles2::GLES2Interface* gl,
unsigned int target, unsigned int target,
......
...@@ -214,6 +214,10 @@ class WebMediaPlayer { ...@@ -214,6 +214,10 @@ class WebMediaPlayer {
virtual uint64_t AudioDecodedByteCount() const = 0; virtual uint64_t AudioDecodedByteCount() const = 0;
virtual uint64_t VideoDecodedByteCount() const = 0; virtual uint64_t VideoDecodedByteCount() const = 0;
// Returns true if the player has a frame available for presentation. Usually
// this just means the first frame has been delivered.
virtual bool HasAvailableVideoFrame() const = 0;
// |already_uploaded_id| indicates the unique_id of the frame last uploaded // |already_uploaded_id| indicates the unique_id of the frame last uploaded
// to this destination. It should only be set by the caller if the contents // to this destination. It should only be set by the caller if the contents
// of the destination are known not to have changed since that upload. // of the destination are known not to have changed since that upload.
......
...@@ -387,7 +387,7 @@ bool HTMLVideoElement::IsPersistent() const { ...@@ -387,7 +387,7 @@ bool HTMLVideoElement::IsPersistent() const {
} }
void HTMLVideoElement::UpdateDisplayState() { void HTMLVideoElement::UpdateDisplayState() {
if (PosterImageURL().IsEmpty()) if (PosterImageURL().IsEmpty() || HasAvailableVideoFrame())
SetDisplayMode(kVideo); SetDisplayMode(kVideo);
else if (GetDisplayMode() < kPoster) else if (GetDisplayMode() < kPoster)
SetDisplayMode(kPoster); SetDisplayMode(kPoster);
...@@ -516,13 +516,9 @@ bool HTMLVideoElement::PrepareVideoFrameForWebGL( ...@@ -516,13 +516,9 @@ bool HTMLVideoElement::PrepareVideoFrameForWebGL(
} }
bool HTMLVideoElement::HasAvailableVideoFrame() const { bool HTMLVideoElement::HasAvailableVideoFrame() const {
if (!GetWebMediaPlayer()) if (auto* wmp = GetWebMediaPlayer())
return false; return wmp->HasAvailableVideoFrame();
return false;
// The ready state maximum is used here instead of the current ready state
// since a frame is still available during a seek.
return GetWebMediaPlayer()->HasVideo() &&
ready_state_maximum_ >= kHaveCurrentData;
} }
void HTMLVideoElement::webkitEnterFullscreen() { void HTMLVideoElement::webkitEnterFullscreen() {
......
...@@ -20,12 +20,15 @@ ...@@ -20,12 +20,15 @@
#include "third_party/blink/renderer/platform/testing/empty_web_media_player.h" #include "third_party/blink/renderer/platform/testing/empty_web_media_player.h"
#include "third_party/blink/renderer/platform/testing/unit_test_helpers.h" #include "third_party/blink/renderer/platform/testing/unit_test_helpers.h"
using testing::_;
namespace blink { namespace blink {
class HTMLVideoElementMockMediaPlayer : public EmptyWebMediaPlayer { class HTMLVideoElementMockMediaPlayer : public EmptyWebMediaPlayer {
public: public:
MOCK_METHOD1(SetIsEffectivelyFullscreen, void(WebFullscreenVideoStatus)); MOCK_METHOD1(SetIsEffectivelyFullscreen, void(WebFullscreenVideoStatus));
MOCK_METHOD1(OnDisplayTypeChanged, void(WebMediaPlayer::DisplayType)); MOCK_METHOD1(OnDisplayTypeChanged, void(WebMediaPlayer::DisplayType));
MOCK_CONST_METHOD0(HasAvailableVideoFrame, bool());
}; };
class HTMLVideoElementTest : public PageTestBase { class HTMLVideoElementTest : public PageTestBase {
...@@ -71,6 +74,7 @@ TEST_F(HTMLVideoElementTest, PictureInPictureInterstitialAndTextContainer) { ...@@ -71,6 +74,7 @@ TEST_F(HTMLVideoElementTest, PictureInPictureInterstitialAndTextContainer) {
video()->UpdateTextTrackDisplay(); video()->UpdateTextTrackDisplay();
// Simulate entering Picture-in-Picture. // Simulate entering Picture-in-Picture.
EXPECT_CALL(*MockWebMediaPlayer(), OnDisplayTypeChanged(_));
video()->OnEnteredPictureInPicture(); video()->OnEnteredPictureInPicture();
// Simulate that text track are displayed again. // Simulate that text track are displayed again.
...@@ -90,6 +94,10 @@ TEST_F(HTMLVideoElementTest, PictureInPictureInterstitial_Reattach) { ...@@ -90,6 +94,10 @@ TEST_F(HTMLVideoElementTest, PictureInPictureInterstitial_Reattach) {
video()->SetSrc("http://example.com/foo.mp4"); video()->SetSrc("http://example.com/foo.mp4");
test::RunPendingTasks(); test::RunPendingTasks();
EXPECT_CALL(*MockWebMediaPlayer(), OnDisplayTypeChanged(_));
EXPECT_CALL(*MockWebMediaPlayer(), HasAvailableVideoFrame())
.WillRepeatedly(testing::Return(true));
// Simulate entering Picture-in-Picture. // Simulate entering Picture-in-Picture.
video()->OnEnteredPictureInPicture(); video()->OnEnteredPictureInPicture();
...@@ -100,6 +108,10 @@ TEST_F(HTMLVideoElementTest, PictureInPictureInterstitial_Reattach) { ...@@ -100,6 +108,10 @@ TEST_F(HTMLVideoElementTest, PictureInPictureInterstitial_Reattach) {
} }
TEST_F(HTMLVideoElementTest, EffectivelyFullscreen_DisplayType) { TEST_F(HTMLVideoElementTest, EffectivelyFullscreen_DisplayType) {
video()->SetSrc("http://example.com/foo.mp4");
test::RunPendingTasks();
UpdateAllLifecyclePhasesForTest();
EXPECT_EQ(WebMediaPlayer::DisplayType::kInline, video()->DisplayType()); EXPECT_EQ(WebMediaPlayer::DisplayType::kInline, video()->DisplayType());
// Vector of data to use for tests. First value is to be set when calling // Vector of data to use for tests. First value is to be set when calling
...@@ -125,6 +137,7 @@ TEST_F(HTMLVideoElementTest, EffectivelyFullscreen_DisplayType) { ...@@ -125,6 +137,7 @@ TEST_F(HTMLVideoElementTest, EffectivelyFullscreen_DisplayType) {
video()->SetIsEffectivelyFullscreen(test.first); video()->SetIsEffectivelyFullscreen(test.first);
EXPECT_EQ(test.second, video()->DisplayType()); EXPECT_EQ(test.second, video()->DisplayType());
testing::Mock::VerifyAndClearExpectations(MockWebMediaPlayer());
} }
} }
...@@ -156,4 +169,16 @@ TEST_F(HTMLVideoElementTest, ChangeLayerNeedsCompositingUpdate) { ...@@ -156,4 +169,16 @@ TEST_F(HTMLVideoElementTest, ChangeLayerNeedsCompositingUpdate) {
EXPECT_FALSE(paint_layer->NeedsCompositingInputsUpdate()); EXPECT_FALSE(paint_layer->NeedsCompositingInputsUpdate());
} }
TEST_F(HTMLVideoElementTest, HasAvailableVideoFrameChecksWMP) {
video()->SetSrc("http://example.com/foo.mp4");
test::RunPendingTasks();
UpdateAllLifecyclePhasesForTest();
EXPECT_CALL(*MockWebMediaPlayer(), HasAvailableVideoFrame())
.WillOnce(testing::Return(false))
.WillOnce(testing::Return(true));
EXPECT_FALSE(video()->HasAvailableVideoFrame());
EXPECT_TRUE(video()->HasAvailableVideoFrame());
}
} // namespace blink } // namespace blink
...@@ -127,7 +127,6 @@ void HtmlVideoElementCapturerSource::sendNewFrame() { ...@@ -127,7 +127,6 @@ void HtmlVideoElementCapturerSource::sendNewFrame() {
const blink::WebSize resolution = web_media_player_->NaturalSize(); const blink::WebSize resolution = web_media_player_->NaturalSize();
if (!canvas_ || is_opaque_ != web_media_player_->IsOpaque()) { if (!canvas_ || is_opaque_ != web_media_player_->IsOpaque()) {
LOG(ERROR) << " Change in opacity !!!";
is_opaque_ = web_media_player_->IsOpaque(); is_opaque_ = web_media_player_->IsOpaque();
if (!bitmap_.tryAllocPixels(SkImageInfo::MakeN32( if (!bitmap_.tryAllocPixels(SkImageInfo::MakeN32(
resolution.width, resolution.height, resolution.width, resolution.height,
......
...@@ -91,6 +91,7 @@ class MockWebMediaPlayer : public WebMediaPlayer { ...@@ -91,6 +91,7 @@ class MockWebMediaPlayer : public WebMediaPlayer {
return; return;
} }
bool IsOpaque() const override { return is_video_opaque_; } bool IsOpaque() const override { return is_video_opaque_; }
bool HasAvailableVideoFrame() const override { return true; }
base::WeakPtr<WebMediaPlayer> AsWeakPtr() override { base::WeakPtr<WebMediaPlayer> AsWeakPtr() override {
return weak_factory_.GetWeakPtr(); return weak_factory_.GetWeakPtr();
......
...@@ -61,6 +61,7 @@ class EmptyWebMediaPlayer : public WebMediaPlayer { ...@@ -61,6 +61,7 @@ class EmptyWebMediaPlayer : public WebMediaPlayer {
cc::PaintFlags&, cc::PaintFlags&,
int already_uploaded_id, int already_uploaded_id,
VideoFrameUploadMetadata*) override {} VideoFrameUploadMetadata*) override {}
bool HasAvailableVideoFrame() const override { return false; }
base::WeakPtr<WebMediaPlayer> AsWeakPtr() override { return nullptr; } base::WeakPtr<WebMediaPlayer> AsWeakPtr() override { return nullptr; }
}; };
......
...@@ -51,6 +51,7 @@ var t = async_test('createImageBitmap(HTMLVideoElement) with resize option'); ...@@ -51,6 +51,7 @@ var t = async_test('createImageBitmap(HTMLVideoElement) with resize option');
// HTMLVideoElement // HTMLVideoElement
var video = document.createElement("video"); var video = document.createElement("video");
video.preload = "auto";
video.oncanplaythrough = t.step_func(function() { video.oncanplaythrough = t.step_func(function() {
return generateTest(); return generateTest();
}); });
......
...@@ -51,6 +51,7 @@ var t = async_test('createImageBitmap(HTMLVideoElement) with resize option'); ...@@ -51,6 +51,7 @@ var t = async_test('createImageBitmap(HTMLVideoElement) with resize option');
// HTMLVideoElement // HTMLVideoElement
var video = document.createElement("video"); var video = document.createElement("video");
video.preload = "auto";
video.oncanplaythrough = t.step_func(function() { video.oncanplaythrough = t.step_func(function() {
return generateTest(); return generateTest();
}); });
......
...@@ -114,6 +114,7 @@ testImage = function () ...@@ -114,6 +114,7 @@ testImage = function ()
testVideo = function () testVideo = function ()
{ {
var video = document.createElement('video'); var video = document.createElement('video');
video.preload = "auto";
video.oncanplay = testResource.bind(null, video, "video", finishUp); video.oncanplay = testResource.bind(null, video, "video", finishUp);
// No crossOrigin set here either. // No crossOrigin set here either.
var name = "../../media/resources/test.ogv"; var name = "../../media/resources/test.ogv";
......
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