Remove HasAudio(), HasVideo(), GetInitialNaturalSize() from media::Pipeline.

content/renderer/media/webmediaplayer_impl.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/renderer/media/webmediaplayer_impl.h"
 
 #include <algorithm>
 #include <limits>
 #include <string>
 #include <vector>
@@ skipping 132 matching lines @@
     base::WeakPtr<WebMediaPlayerDelegate> delegate,
     const WebMediaPlayerParams& params)
     : frame_(frame),
       network_state_(WebMediaPlayer::NetworkStateEmpty),
       ready_state_(WebMediaPlayer::ReadyStateHaveNothing),
       main_loop_(base::MessageLoopProxy::current()),
       media_loop_(
           RenderThreadImpl::current()->GetMediaThreadMessageLoopProxy()),
       media_log_(new RenderMediaLog()),
       pipeline_(media_loop_, media_log_.get()),
+      has_audio_(false),
+      has_video_(false),
       paused_(true),
       seeking_(false),
       playback_rate_(0.0f),
       pending_seek_(false),
       pending_seek_seconds_(0.0f),
       client_(client),
       delegate_(delegate),
       defer_load_cb_(params.defer_load_cb()),
       accelerated_compositing_reported_(false),
       incremented_externally_allocated_memory_(false),
@@ skipping 266 matching lines @@
 void WebMediaPlayerImpl::setPreload(WebMediaPlayer::Preload preload) {
   DCHECK(main_loop_->BelongsToCurrentThread());
 
   if (data_source_)
     data_source_->SetPreload(static_cast<content::Preload>(preload));
 }
 
 bool WebMediaPlayerImpl::hasVideo() const {
   DCHECK(main_loop_->BelongsToCurrentThread());
 
-  return pipeline_.HasVideo();
+  return has_video_;
 }
 
 bool WebMediaPlayerImpl::hasAudio() const {
   DCHECK(main_loop_->BelongsToCurrentThread());
 
-  return pipeline_.HasAudio();
+  return has_audio_;
 }
 
 blink::WebSize WebMediaPlayerImpl::naturalSize() const {
   DCHECK(main_loop_->BelongsToCurrentThread());
 
   return blink::WebSize(natural_size_);
 }
 
 bool WebMediaPlayerImpl::paused() const {
   DCHECK(main_loop_->BelongsToCurrentThread());
@@ skipping 505 matching lines @@
   SetNetworkState(PipelineErrorToNetworkState(error));
 
   if (error == media::PIPELINE_ERROR_DECRYPT)
     EmeUMAHistogramCounts(current_key_system_, "DecryptError", 1);
 
   // TODO(scherkus): This should be handled by HTMLMediaElement and controls
   // should know when to invalidate themselves http://crbug.com/337015
   InvalidateOnMainThread();
 }
 
+void WebMediaPlayerImpl::OnPipelineHasTrack(
+    media::Pipeline::TrackType track) {
+  switch (track) {
+    case media::Pipeline::AUDIO:
+      has_audio_ = true;
+      break;
+
+    case media::Pipeline::VIDEO:
+      has_video_ = true;
+      break;
+  }
+}
+
 void WebMediaPlayerImpl::OnPipelineBufferingState(
     media::Pipeline::BufferingState buffering_state) {
   DVLOG(1) << "OnPipelineBufferingState(" << buffering_state << ")";
 
   switch (buffering_state) {
     case media::Pipeline::kHaveMetadata:
       // TODO(scherkus): Would be better to have a metadata changed callback
       // that contained the size information as well whether audio/video is
       // present. Doing so would let us remove more methods off Pipeline.
       natural_size_ = pipeline_.GetInitialNaturalSize();

[scherkus (not reviewing), 2014/03/20 23:16:23]

ahh this is the TODO I was looking for!

it seems like we should combine what you have here w/ this TODO to have a
dedicated HaveMetadataCB that can return, say, a PipelineMetadata struct that
contains the following:

struct PipelineMetadata {
  bool has_audio_;
  bool has_video_;
  gfx::Rect natural_size_;
}

... but since that'll introduce a lot of churn for this CL, let's do the
following:
  1) Define that struct (could be an inner class of Pipeline, but we tend to
avoid inner classes since you can't forward declare them so I'd stick it in the
top of pipeline.h)
  2) Update BufferingStateCB to include that struct as a second parameter
  3) Fill out the metadata struct and run the buffering_state_cb_ around line
~468 in pipeline.cc
  4) Nuke Pipeline::GetInitialNaturalSize()

this way we'll get all our metadata in a single shot. you can even have WMPI
have a local copy of PipelineMetadata instead of separate has_audio_ /
has_video_ / natural_size_ members.

[sandersd (OOO until July 31), 2014/03/21 19:53:03]

Done.
I tried this in patch set 2, but it seems better to split it out into two
callbacks, which I've done in patch set 3.
Done and done.

I'm not sure I got all of the locking correct, so PTAL at that.

 
       SetReadyState(WebMediaPlayer::ReadyStateHaveMetadata);
 
       if (hasVideo() && client_->needsWebLayerForVideo()) {
         DCHECK(!video_weblayer_);
         video_weblayer_.reset(
             new webkit::WebLayerImpl(cc::VideoLayer::Create(this)));
         client_->setWebLayer(video_weblayer_.get());
       }
       break;
@@ skipping 222 matching lines @@
     filter_collection->SetTextRenderer(text_renderer.Pass());
   }
 
   // ... and we're ready to go!
   starting_ = true;
   pipeline_.Start(
       filter_collection.Pass(),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineEnded),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineError),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineSeek),
+      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineHasTrack),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineBufferingState),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnDurationChange));
 }
 
 void WebMediaPlayerImpl::SetNetworkState(WebMediaPlayer::NetworkState state) {
   DCHECK(main_loop_->BelongsToCurrentThread());
   DVLOG(1) << "SetNetworkState: " << state;
   network_state_ = state;
   // Always notify to ensure client has the latest value.
   client_->networkStateChanged();
@@ skipping 90 matching lines @@
 
   if (web_cdm_) {
     decryptor_ready_cb.Run(web_cdm_->GetDecryptor());
     return;
   }
 
   decryptor_ready_cb_ = decryptor_ready_cb;
 }
 
 }  // namespace content