Commit 6fab02da authored by Dan Sanders's avatar Dan Sanders Committed by Commit Bot

[media] Clarify PipelineImpl locking comments.

Bug: 893739
Change-Id: Icb19a36549d9f877f5b32a5a10b05f1420f812a8
Reviewed-on: https://chromium-review.googlesource.com/c/1279292
Commit-Queue: Dan Sanders <sandersd@chromium.org>
Reviewed-by: default avatarJohn Rummell <jrummell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599882}
parent 1e0704e6
...@@ -80,9 +80,10 @@ class PipelineImpl::RendererWrapper : public DemuxerHost, ...@@ -80,9 +80,10 @@ class PipelineImpl::RendererWrapper : public DemuxerHost,
base::OnceClosure change_completed_cb); base::OnceClosure change_completed_cb);
private: private:
// Contains state shared between main and media thread. // Contains state shared between main and media thread. On the media thread
// Main thread can only read. Media thread can both - read and write. // each member can be read without locking, but writing requires locking. On
// So it is not necessary to lock when reading from the media thread. // the main thread reading requires a lock and writing is prohibited.
//
// This struct should only contain state that is not immediately needed by // This struct should only contain state that is not immediately needed by
// PipelineClient and can be cached on the media thread until queried. // PipelineClient and can be cached on the media thread until queried.
// Alternatively we could cache it on the main thread by posting the // Alternatively we could cache it on the main thread by posting the
...@@ -93,6 +94,10 @@ class PipelineImpl::RendererWrapper : public DemuxerHost, ...@@ -93,6 +94,10 @@ class PipelineImpl::RendererWrapper : public DemuxerHost,
struct SharedState { struct SharedState {
// TODO(scherkus): Enforce that Renderer is only called on a single thread, // TODO(scherkus): Enforce that Renderer is only called on a single thread,
// even for accessing media time http://crbug.com/370634 // even for accessing media time http://crbug.com/370634
//
// Note: Renderer implementations must support GetMediaTime() being called
// on both the main and media threads. RendererWrapper::GetMediaTime() calls
// it from the main thread (locked).
std::unique_ptr<Renderer> renderer; std::unique_ptr<Renderer> renderer;
// True when OnBufferedTimeRangesChanged() has been called more recently // True when OnBufferedTimeRangesChanged() has been called more recently
...@@ -536,6 +541,7 @@ void PipelineImpl::RendererWrapper::OnEnded() { ...@@ -536,6 +541,7 @@ void PipelineImpl::RendererWrapper::OnEnded() {
// TODO(crbug/817089): Combine this functionality into renderer->GetMediaTime(). // TODO(crbug/817089): Combine this functionality into renderer->GetMediaTime().
base::TimeDelta PipelineImpl::RendererWrapper::GetCurrentTimestamp() { base::TimeDelta PipelineImpl::RendererWrapper::GetCurrentTimestamp() {
DCHECK(media_task_runner_->BelongsToCurrentThread());
DCHECK(demuxer_); DCHECK(demuxer_);
DCHECK(shared_state_.renderer || state_ != kPlaying); DCHECK(shared_state_.renderer || state_ != kPlaying);
......
...@@ -56,6 +56,8 @@ class MEDIA_EXPORT Renderer { ...@@ -56,6 +56,8 @@ class MEDIA_EXPORT Renderer {
virtual void SetVolume(float volume) = 0; virtual void SetVolume(float volume) = 0;
// Returns the current media time. // Returns the current media time.
//
// This method must be safe to call from any thread.
virtual base::TimeDelta GetMediaTime() = 0; virtual base::TimeDelta GetMediaTime() = 0;
// Provides a list of DemuxerStreams correlating to the tracks which should // Provides a list of DemuxerStreams correlating to the tracks which should
......
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