Skip to content

Commit

Permalink
Revert of Prime the landing pad for the new video rendering pipeline.…
Browse files Browse the repository at this point in the history
… (patchset chromium#9 id:200001 of https://codereview.chromium.org/1053113002/)

Reason for revert:
Actually, given the complexity of this change (not a one-line Cast fix) and the fact that we have broken Cast trybots running @ 100% right now (http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linux/), I'm going to revert this. I'm working on a brief patch of what I expect would need to be done for Cast and will upload/mail shortly.

Original issue's description:
> Prime the landing pad for the new video rendering pipeline.
>
> This is not a functional change, it only updates the interfaces and
> call sites in preparation for switching to a vsync based video
> rendering pipeline.
>
> Some notes:
> - Plumbs a VideoRendererSink into the the rendering pipeline; similar to
> how we have an AudioRendererSink.
> - A couple VideoRendererSink mocks are introduced which will be short
> lived. Like audio, we will need fakes which can pump consumption tasks.
> - The "PaintCB" callback has been temporarily placed on the new sink
> interface such that in the field experiments can be run comparing the
> performance of the video rendering approaches.
> - Finally nukes Player_X11 since setting up a vsync renderer just for
> unused tool code isn't worth the effort.
> - Since compositor callbacks may stop due to visibility changes, the
> new VideoRendererImpl will use a countdown timer to pump video playback
> as frames expire; expired frames will not count as dropped.
> - Since canvas/WebGL requires frame updates in the background a new
> method has been added to VideoFrameCompositor to return the current
> frame if it was updated with 250ms, or to request a new one and return
> the updated one.
>
> Subsequent work:
> - sunnyps@ will be switching VideoFrameProviderClientImpl over to using
> a BeginFrameObserver, which will ultimately drive the Render() callbacks.
> - dalecurtis@ will land the VideoRendererAlgorithm which powers the new
> rendering pipeline.
>
> BUG=439548
> TEST=everything works as is.
>
> Committed: https://crrev.com/e7f41df2541aea7c99f7965874f9c5ce901899e5
> Cr-Commit-Position: refs/heads/master@{#325183}

TBR=xhwang@chromium.org,sunnyps@chromium.org,brianderson@chromium.org,enne@chromium.org,dpranke@chromium.org,danakj@chromium.org,dalecurtis@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=439548

Review URL: https://codereview.chromium.org/1052493005

Cr-Commit-Position: refs/heads/master@{#325187}
  • Loading branch information
gunsch authored and Commit bot committed Apr 15, 2015
1 parent ff64870 commit 24413fd
Show file tree
Hide file tree
Showing 53 changed files with 1,221 additions and 481 deletions.
1 change: 1 addition & 0 deletions BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,7 @@ group("gn_all") {
}

if (use_x11) {
deps += [ "//media:player_x11" ]
if (target_cpu != "arm") {
deps += [ "//gpu/tools/compositor_model_bench" ]
}
Expand Down
1 change: 1 addition & 0 deletions build/gn_migration.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@

['use_x11==1', {
'dependencies': [
'../media/media.gyp:player_x11',
'../tools/xdisplaycheck/xdisplaycheck.gyp:xdisplaycheck',
],
'conditions': [
Expand Down
70 changes: 21 additions & 49 deletions cc/layers/video_frame_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
#define CC_LAYERS_VIDEO_FRAME_PROVIDER_H_

#include "base/memory/ref_counted.h"
#include "base/time/time.h"
#include "cc/base/cc_export.h"

namespace media {
Expand All @@ -15,39 +14,27 @@ class VideoFrame;

namespace cc {

// VideoFrameProvider and VideoFrameProvider::Client define the relationship by
// which video frames are exchanged between a provider and client.
//
// Threading notes: This class may be used in a multithreaded manner. However,
// if the Client implementation calls GetCurrentFrame()/PutCurrentFrame() from
// one thread, the provider must ensure that all client methods (except
// StopUsingProvider()) are called from that thread (typically the compositor
// thread).
// Threading notes: This class may be used in a multi threaded manner.
// Specifically, the implementation may call GetCurrentFrame() or
// PutCurrentFrame() from the compositor thread. If so, the caller is
// responsible for making sure Client::DidReceiveFrame() and
// Client::DidUpdateMatrix() are only called from this same thread.
class CC_EXPORT VideoFrameProvider {
public:
virtual ~VideoFrameProvider() {}

class CC_EXPORT Client {
public:
// The provider will call this method to tell the client to stop using it.
// Provider will call this method to tell the client to stop using it.
// StopUsingProvider() may be called from any thread. The client should
// block until it has PutCurrentFrame() any outstanding frames.
virtual void StopUsingProvider() = 0;

// Notifies the client that it should start or stop making regular
// UpdateCurrentFrame() calls to the provider. No further calls to
// UpdateCurrentFrame() should be made once StopRendering() returns.
//
// Callers should use these methods to indicate when it expects and no
// longer expects (respectively) to have new frames for the client. Clients
// may use this information for power conservation.
virtual void StartRendering() = 0;
virtual void StopRendering() = 0;

// Notifies the client that GetCurrentFrame() will return new data.
// TODO(dalecurtis): Nuke this once VideoFrameProviderClientImpl is using a
// BeginFrameObserver based approach. http://crbug.com/336733
// Notifies the provider's client that a call to GetCurrentFrame() will
// return new data.
virtual void DidReceiveFrame() = 0;

// Notifies the client of a new UV transform matrix to be used.
// Notifies the provider's client of a new UV transform matrix to be used.
virtual void DidUpdateMatrix(const float* matrix) = 0;

protected:
Expand All @@ -58,33 +45,18 @@ class CC_EXPORT VideoFrameProvider {
// that the provider is not destroyed before this call returns.
virtual void SetVideoFrameProviderClient(Client* client) = 0;

// Called by the client on a regular interval. Returns true if a new frame
// will be available via GetCurrentFrame() which should be displayed within
// the presentation interval [|deadline_min|, |deadline_max|].
//
// Implementations may use this to drive frame acquisition from underlying
// sources, so it must be called by clients before calling GetCurrentFrame().
virtual bool UpdateCurrentFrame(base::TimeTicks deadline_min,
base::TimeTicks deadline_max) = 0;

// Returns the current frame, which may have been updated by a recent call to
// UpdateCurrentFrame(). A call to this method does not ensure that the frame
// will be rendered. A subsequent call to PutCurrentFrame() must be made if
// the frame is expected to be rendered.
//
// Clients should call this in response to UpdateCurrentFrame() returning true
// or in response to a DidReceiveFrame() call.
//
// TODO(dalecurtis): Remove text about DidReceiveFrame() once the old path
// has been removed. http://crbug.com/439548
// This function places a lock on the current frame and returns a pointer to
// it. Calls to this method should always be followed with a call to
// PutCurrentFrame().
// Only the current provider client should call this function.
virtual scoped_refptr<media::VideoFrame> GetCurrentFrame() = 0;

// Indicates that the last frame returned via GetCurrentFrame() is expected to
// be rendered. Must only occur after a previous call to GetCurrentFrame().
virtual void PutCurrentFrame() = 0;

protected:
virtual ~VideoFrameProvider() {}
// This function releases the lock on the video frame. It should always be
// called after GetCurrentFrame(). Frames passed into this method
// should no longer be referenced after the call is made. Only the current
// provider client should call this function.
virtual void PutCurrentFrame(
const scoped_refptr<media::VideoFrame>& frame) = 0;
};

} // namespace cc
Expand Down
15 changes: 3 additions & 12 deletions cc/layers/video_frame_provider_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,11 @@ VideoFrameProviderClientImpl::AcquireLockAndCurrentFrame() {
return provider_->GetCurrentFrame();
}

void VideoFrameProviderClientImpl::PutCurrentFrame() {
void VideoFrameProviderClientImpl::PutCurrentFrame(
const scoped_refptr<media::VideoFrame>& frame) {
DCHECK(thread_checker_.CalledOnValidThread());
provider_lock_.AssertAcquired();
provider_->PutCurrentFrame();
provider_->PutCurrentFrame(frame);
}

void VideoFrameProviderClientImpl::ReleaseLock() {
Expand All @@ -103,16 +104,6 @@ void VideoFrameProviderClientImpl::StopUsingProvider() {
provider_ = nullptr;
}

void VideoFrameProviderClientImpl::StartRendering() {
// TODO(dalecurtis, sunnyps): Hook this method up to control when to start
// observing vsync intervals. http://crbug.com/336733
}

void VideoFrameProviderClientImpl::StopRendering() {
// TODO(dalecurtis, sunnyps): Hook this method up to control when to stop
// observing vsync intervals. http://crbug.com/336733
}

void VideoFrameProviderClientImpl::DidReceiveFrame() {
TRACE_EVENT1("cc",
"VideoFrameProviderClientImpl::DidReceiveFrame",
Expand Down
4 changes: 1 addition & 3 deletions cc/layers/video_frame_provider_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class CC_EXPORT VideoFrameProviderClientImpl
void Stop();

scoped_refptr<media::VideoFrame> AcquireLockAndCurrentFrame();
void PutCurrentFrame();
void PutCurrentFrame(const scoped_refptr<media::VideoFrame>& frame);
void ReleaseLock();

const gfx::Transform& StreamTextureMatrix() const;
Expand All @@ -46,8 +46,6 @@ class CC_EXPORT VideoFrameProviderClientImpl
// Called on the main thread.
void StopUsingProvider() override;
// Called on the impl thread.
void StartRendering() override;
void StopRendering() override;
void DidReceiveFrame() override;
void DidUpdateMatrix(const float* matrix) override;

Expand Down
2 changes: 1 addition & 1 deletion cc/layers/video_layer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ void VideoLayerImpl::DidDraw(ResourceProvider* resource_provider) {
frame_resources_.clear();
}

provider_client_impl_->PutCurrentFrame();
provider_client_impl_->PutCurrentFrame(frame_);
frame_ = nullptr;

provider_client_impl_->ReleaseLock();
Expand Down
5 changes: 0 additions & 5 deletions cc/test/fake_video_frame_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ FakeVideoFrameProvider::~FakeVideoFrameProvider() {
client_->StopUsingProvider();
}

bool FakeVideoFrameProvider::UpdateCurrentFrame(base::TimeTicks deadline_min,
base::TimeTicks deadline_max) {
return false;
}

void FakeVideoFrameProvider::SetVideoFrameProviderClient(Client* client) {
client_ = client;
}
Expand Down
4 changes: 1 addition & 3 deletions cc/test/fake_video_frame_provider.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,8 @@ class FakeVideoFrameProvider : public VideoFrameProvider {
~FakeVideoFrameProvider() override;

void SetVideoFrameProviderClient(Client* client) override;
bool UpdateCurrentFrame(base::TimeTicks deadline_min,
base::TimeTicks deadline_max) override;
scoped_refptr<media::VideoFrame> GetCurrentFrame() override;
void PutCurrentFrame() override {}
void PutCurrentFrame(const scoped_refptr<media::VideoFrame>&) override {}

Client* client() { return client_; }

Expand Down
9 changes: 2 additions & 7 deletions content/renderer/media/android/webmediaplayer_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1272,12 +1272,6 @@ void WebMediaPlayerAndroid::SetCurrentFrameInternal(
current_frame_ = video_frame;
}

bool WebMediaPlayerAndroid::UpdateCurrentFrame(base::TimeTicks deadline_min,
base::TimeTicks deadline_max) {
NOTIMPLEMENTED();
return false;
}

scoped_refptr<media::VideoFrame> WebMediaPlayerAndroid::GetCurrentFrame() {
scoped_refptr<VideoFrame> video_frame;
{
Expand All @@ -1288,7 +1282,8 @@ scoped_refptr<media::VideoFrame> WebMediaPlayerAndroid::GetCurrentFrame() {
return video_frame;
}

void WebMediaPlayerAndroid::PutCurrentFrame() {
void WebMediaPlayerAndroid::PutCurrentFrame(
const scoped_refptr<media::VideoFrame>& frame) {
}

void WebMediaPlayerAndroid::ResetStreamTextureProxy() {
Expand Down
4 changes: 1 addition & 3 deletions content/renderer/media/android/webmediaplayer_android.h
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,8 @@ class WebMediaPlayerAndroid : public blink::WebMediaPlayer,
// compositor thread.
void SetVideoFrameProviderClient(
cc::VideoFrameProvider::Client* client) override;
bool UpdateCurrentFrame(base::TimeTicks deadline_min,
base::TimeTicks deadline_max) override;
scoped_refptr<media::VideoFrame> GetCurrentFrame() override;
void PutCurrentFrame() override;
void PutCurrentFrame(const scoped_refptr<media::VideoFrame>& frame) override;

// Media player callback handlers.
void OnMediaMetadataChanged(const base::TimeDelta& duration, int width,
Expand Down
11 changes: 2 additions & 9 deletions content/renderer/media/webmediaplayer_ms.cc
Original file line number Diff line number Diff line change
Expand Up @@ -456,14 +456,6 @@ void WebMediaPlayerMS::SetVideoFrameProviderClient(
video_frame_provider_client_ = client;
}

bool WebMediaPlayerMS::UpdateCurrentFrame(base::TimeTicks deadline_min,
base::TimeTicks deadline_max) {
// TODO(dalecurtis): This should make use of the deadline interval to ensure
// the painted frame is correct for the given interval.
NOTREACHED();
return false;
}

scoped_refptr<media::VideoFrame> WebMediaPlayerMS::GetCurrentFrame() {
DVLOG(3) << "WebMediaPlayerMS::GetCurrentFrame";
base::AutoLock auto_lock(current_frame_lock_);
Expand All @@ -475,7 +467,8 @@ scoped_refptr<media::VideoFrame> WebMediaPlayerMS::GetCurrentFrame() {
return current_frame_;
}

void WebMediaPlayerMS::PutCurrentFrame() {
void WebMediaPlayerMS::PutCurrentFrame(
const scoped_refptr<media::VideoFrame>& frame) {
DVLOG(3) << "WebMediaPlayerMS::PutCurrentFrame";
DCHECK(pending_repaint_);
pending_repaint_ = false;
Expand Down
4 changes: 1 addition & 3 deletions content/renderer/media/webmediaplayer_ms.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,8 @@ class WebMediaPlayerMS
// VideoFrameProvider implementation.
void SetVideoFrameProviderClient(
cc::VideoFrameProvider::Client* client) override;
bool UpdateCurrentFrame(base::TimeTicks deadline_min,
base::TimeTicks deadline_max) override;
scoped_refptr<media::VideoFrame> GetCurrentFrame() override;
void PutCurrentFrame() override;
void PutCurrentFrame(const scoped_refptr<media::VideoFrame>& frame) override;

private:
// The callback for VideoFrameProvider to signal a new frame is available.
Expand Down
31 changes: 31 additions & 0 deletions media/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -792,3 +792,34 @@ if (media_use_ffmpeg) {
]
}
}

if (use_x11) {
executable("player_x11") {
sources = [
"tools/player_x11/data_source_logger.cc",
"tools/player_x11/data_source_logger.h",
"tools/player_x11/gl_video_renderer.cc",
"tools/player_x11/gl_video_renderer.h",
"tools/player_x11/player_x11.cc",
"tools/player_x11/x11_video_renderer.cc",
"tools/player_x11/x11_video_renderer.h",
]
configs += [
":media_config",
"//build/config/linux:x11",
"//build/config/linux:xext",

# TODO(ajwong): Why does xext get a separate thing in //build/config/linux:BUILD.gn
# "//build/config/linux:xrender",
]
deps = [
":media",
":shared_memory_support",
"//base",
"//tools/xdisplaycheck",
"//ui/gl",
"//ui/gfx",
"//ui/gfx/geometry",
]
}
}
24 changes: 13 additions & 11 deletions media/base/mock_filters.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,16 +121,17 @@ class MockVideoRenderer : public VideoRenderer {
virtual ~MockVideoRenderer();

// VideoRenderer implementation.
MOCK_METHOD9(Initialize,
void(DemuxerStream* stream,
const PipelineStatusCB& init_cb,
const SetDecryptorReadyCB& set_decryptor_ready_cb,
const StatisticsCB& statistics_cb,
const BufferingStateCB& buffering_state_cb,
const base::Closure& ended_cb,
const PipelineStatusCB& error_cb,
const WallClockTimeCB& wall_clock_time_cb,
const base::Closure& waiting_for_decryption_key_cb));
MOCK_METHOD10(Initialize,
void(DemuxerStream* stream,
const PipelineStatusCB& init_cb,
const SetDecryptorReadyCB& set_decryptor_ready_cb,
const StatisticsCB& statistics_cb,
const BufferingStateCB& buffering_state_cb,
const PaintCB& paint_cb,
const base::Closure& ended_cb,
const PipelineStatusCB& error_cb,
const WallClockTimeCB& wall_clock_time_cb,
const base::Closure& waiting_for_decryption_key_cb));
MOCK_METHOD1(Flush, void(const base::Closure& callback));
MOCK_METHOD1(StartPlayingFrom, void(base::TimeDelta));
MOCK_METHOD1(OnTimeStateChanged, void(bool));
Expand Down Expand Up @@ -169,11 +170,12 @@ class MockRenderer : public Renderer {
virtual ~MockRenderer();

// Renderer implementation.
MOCK_METHOD7(Initialize,
MOCK_METHOD8(Initialize,
void(DemuxerStreamProvider* demuxer_stream_provider,
const PipelineStatusCB& init_cb,
const StatisticsCB& statistics_cb,
const BufferingStateCB& buffering_state_cb,
const PaintCB& paint_cb,
const base::Closure& ended_cb,
const PipelineStatusCB& error_cb,
const base::Closure& waiting_for_decryption_key_cb));
Expand Down
4 changes: 4 additions & 0 deletions media/base/pipeline.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ void Pipeline::Start(Demuxer* demuxer,
const PipelineStatusCB& seek_cb,
const PipelineMetadataCB& metadata_cb,
const BufferingStateCB& buffering_state_cb,
const PaintCB& paint_cb,
const base::Closure& duration_change_cb,
const AddTextTrackCB& add_text_track_cb,
const base::Closure& waiting_for_decryption_key_cb) {
Expand All @@ -76,6 +77,7 @@ void Pipeline::Start(Demuxer* demuxer,
DCHECK(!seek_cb.is_null());
DCHECK(!metadata_cb.is_null());
DCHECK(!buffering_state_cb.is_null());
DCHECK(!paint_cb.is_null());

base::AutoLock auto_lock(lock_);
CHECK(!running_) << "Media pipeline is already running";
Expand All @@ -88,6 +90,7 @@ void Pipeline::Start(Demuxer* demuxer,
seek_cb_ = seek_cb;
metadata_cb_ = metadata_cb;
buffering_state_cb_ = buffering_state_cb;
paint_cb_ = paint_cb;
duration_change_cb_ = duration_change_cb;
add_text_track_cb_ = add_text_track_cb;
waiting_for_decryption_key_cb_ = waiting_for_decryption_key_cb;
Expand Down Expand Up @@ -711,6 +714,7 @@ void Pipeline::InitializeRenderer(const PipelineStatusCB& done_cb) {
done_cb,
base::Bind(&Pipeline::OnUpdateStatistics, weak_this),
base::Bind(&Pipeline::BufferingStateChanged, weak_this),
base::ResetAndReturn(&paint_cb_),
base::Bind(&Pipeline::OnRendererEnded, weak_this),
base::Bind(&Pipeline::OnError, weak_this),
waiting_for_decryption_key_cb_);
Expand Down
Loading

0 comments on commit 24413fd

Please sign in to comment.