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

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

This splits PipelineBufferingStateCB in two: PipelineMetadataCB, which includes the available tracks and natural size in a PipelineMetadata struct; and PipelinePrerollCompleted, with no parameters. Now WebMediaPlayerImpl can cache the metadata and the accessors are not required.

BUG=148541
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=259154

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

https://codereview.chromium.org/206103004/diff/1/content/renderer/media/webme...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/206103004/diff/1/content/renderer/media/webme...
content/renderer/media/webmediaplayer_impl.cc:995: natural_size_ =
pipeline_.GetInitialNaturalSize();
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]

https://codereview.chromium.org/206103004/diff/1/content/renderer/media/webme...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/206103004/diff/1/content/renderer/media/webme...
content/renderer/media/webmediaplayer_impl.cc:995: natural_size_ =
pipeline_.GetInitialNaturalSize();
>   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)

Done.

>   2) Update BufferingStateCB to include that struct as a second parameter

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.

>   3) Fill out the metadata struct and run the buffering_state_cb_ around line
> ~468 in pipeline.cc
>   4) Nuke Pipeline::GetInitialNaturalSize()

Done and done.

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

[scherkus (not reviewing), 2014-03-21 22:26:10]

looking good!

some nits on naming 'n stuff

https://codereview.chromium.org/206103004/diff/30001/content/renderer/media/w...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/206103004/diff/30001/content/renderer/media/w...
content/renderer/media/webmediaplayer_impl.cc:153: pipeline_metadata_(),
nit: no need -- C++ initializer lists will call this for us

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc
File media/base/pipeline.cc (right):

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:327: if (audio_renderer_ == NULL || audio_disabled_)
audio_renderer_ should never be NULL in this case as we only register this
callback if it exists in InitializeAudioRenderer()

remove from if statement -- you can DCHECK(audio_renderer_) if you want

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:346: if (audio_renderer_ != NULL && !audio_disabled_)
nit: hrmm.. can we ditch the direct comparison to NULL here + in the rest of
your CL?  AFAIK scoped_ptr<T> has bool conversion

(pithiness: treating pointers as bool is less verbose than a comparison to NULL,
so we like to do it whenever possible)

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:465: PipelineMetadata metadata;
what about adding an extra ctor that lets you set all values at once?

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:963: initial_natural_size_ =
stream->video_decoder_config().natural_size();
you can move this up to where we call have_metadata_cb_.Run()

the funny part is that now that we don't need to hold the lock to set
|initial_natural_size| (because your CL makes us use message passing, which is
good!) this whole comment + TODO can get nuked

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h
File media/base/pipeline.h (right):

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:38: // buffering_state callback.
second sentence is incorrect -- heck I'd just remove it since it you cover the
timing aspect by saying "once it has been initialized"

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:40: PipelineMetadata() : has_audio_(), has_video_(),
natural_size_() {}
nit: instead of using the zero-initializer for POD types (e.g., bools) it's
better to be explicit and state false

for class types (e.g., gfx::Size) you can let the initializer list call the
default constructor

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:42: bool has_audio_;
structs don't use trailing _ for variables:
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Variable_Names

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:47: typedef base::Callback<void(PipelineMetadata)>
HaveMetadataCB;
I'd model the naming after PipelineStatusCB:
  - call this PipelineMetadataCB
  - have the variable names metadata_cb_ (pithiness is nice where possible)

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:97: //   |error_cb| optional callback that will be
executed whenever an error
careful tweaking comments: error_cb is definitely non-optional!

(it might look like it since it's guarded by an if-statement, however that's
because we call error_cb_.Reset() to ensure we only call either stop_cb_ or
error_cb_)

also see my comments below regarding optional params

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:100: //   |have_metadata_cb| optional callback that will
be executed when the
I usually advocate for as little optional-ness as possible and to follow what's
needed in production code. It makes APIs stricter and the impl code simpler.

In our case, our production code is WebMediaPlayerImpl where it passes in
callbacks for all of these.


(I know the existing |duration_change_cb| is stated as optional, but the only
caller that passes in an empty callback is pipeline_integration_test.cc. It'd be
nicer to have it pass in something. We should fix that in a separate CL.)

[sandersd (OOO until July 31), 2014-03-21 23:30:34]

I'll get started on the tests right away.

https://codereview.chromium.org/206103004/diff/30001/content/renderer/media/w...
File content/renderer/media/webmediaplayer_impl.cc (right):

https://codereview.chromium.org/206103004/diff/30001/content/renderer/media/w...
content/renderer/media/webmediaplayer_impl.cc:153: pipeline_metadata_(),
On 2014/03/21 22:26:10, scherkus wrote:
> nit: no need -- C++ initializer lists will call this for us

Done.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc
File media/base/pipeline.cc (right):

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:327: if (audio_renderer_ == NULL || audio_disabled_)
On 2014/03/21 22:26:10, scherkus wrote:
> audio_renderer_ should never be NULL in this case as we only register this
> callback if it exists in InitializeAudioRenderer()
> 
> remove from if statement -- you can DCHECK(audio_renderer_) if you want

Done.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:346: if (audio_renderer_ != NULL && !audio_disabled_)
On 2014/03/21 22:26:10, scherkus wrote:
> nit: hrmm.. can we ditch the direct comparison to NULL here + in the rest of
> your CL?  AFAIK scoped_ptr<T> has bool conversion
> 
> (pithiness: treating pointers as bool is less verbose than a comparison to
NULL,
> so we like to do it whenever possible)

Done.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:465: PipelineMetadata metadata;
On 2014/03/21 22:26:10, scherkus wrote:
> what about adding an extra ctor that lets you set all values at once?

That is what I did initially, but I didn't like that there was no natural
ordering for has_audio and has_video.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:963: initial_natural_size_ =
stream->video_decoder_config().natural_size();
On 2014/03/21 22:26:10, scherkus wrote:
> you can move this up to where we call have_metadata_cb_.Run()
> 
> the funny part is that now that we don't need to hold the lock to set
> |initial_natural_size| (because your CL makes us use message passing, which is
> good!) this whole comment + TODO can get nuked

Done.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h
File media/base/pipeline.h (right):

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:38: // buffering_state callback.
On 2014/03/21 22:26:10, scherkus wrote:
> second sentence is incorrect -- heck I'd just remove it since it you cover the
> timing aspect by saying "once it has been initialized"

Done.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:40: PipelineMetadata() : has_audio_(), has_video_(),
natural_size_() {}
On 2014/03/21 22:26:10, scherkus wrote:
> nit: instead of using the zero-initializer for POD types (e.g., bools) it's
> better to be explicit and state false
> 
> for class types (e.g., gfx::Size) you can let the initializer list call the
> default constructor

Done.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:42: bool has_audio_;
On 2014/03/21 22:26:10, scherkus wrote:
> structs don't use trailing _ for variables:
> http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Variable_Names

Done.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:47: typedef base::Callback<void(PipelineMetadata)>
HaveMetadataCB;
On 2014/03/21 22:26:10, scherkus wrote:
> I'd model the naming after PipelineStatusCB:
>   - call this PipelineMetadataCB
>   - have the variable names metadata_cb_ (pithiness is nice where possible)

Done.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:97: //   |error_cb| optional callback that will be
executed whenever an error
On 2014/03/21 22:26:10, scherkus wrote:
> careful tweaking comments: error_cb is definitely non-optional!
> 
> (it might look like it since it's guarded by an if-statement, however that's
> because we call error_cb_.Reset() to ensure we only call either stop_cb_ or
> error_cb_)
> 
> also see my comments below regarding optional params

Done.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.h#ne...
media/base/pipeline.h:100: //   |have_metadata_cb| optional callback that will
be executed when the
On 2014/03/21 22:26:10, scherkus wrote:
> I usually advocate for as little optional-ness as possible and to follow
what's
> needed in production code. It makes APIs stricter and the impl code simpler.
> 
> In our case, our production code is WebMediaPlayerImpl where it passes in
> callbacks for all of these.
> 
> 
> (I know the existing |duration_change_cb| is stated as optional, but the only
> caller that passes in an empty callback is pipeline_integration_test.cc. It'd
be
> nicer to have it pass in something. We should fix that in a separate CL.)

Done.

[scherkus (not reviewing), 2014-03-21 23:36:36]

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc
File media/base/pipeline.cc (right):

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:465: PipelineMetadata metadata;
On 2014/03/21 23:30:35, Dan Sanders wrote:
> On 2014/03/21 22:26:10, scherkus wrote:
> > what about adding an extra ctor that lets you set all values at once?
> 
> That is what I did initially, but I didn't like that there was no natural
> ordering for has_audio and has_video.

out of curiosity can you clarify what you meant by "natural ordering"?

https://codereview.chromium.org/206103004/diff/50001/media/base/pipeline.cc
File media/base/pipeline.cc (right):

https://codereview.chromium.org/206103004/diff/50001/media/base/pipeline.cc#n...
media/base/pipeline.cc:470: gfx::Size natural_size =
stream->video_decoder_config().natural_size();
we should still check whether |stream| exists (the old code could be sure it
existed since that's a precondition of calling InitializeVideoRenderer())

[scherkus (not reviewing), 2014-03-21 23:38:37]

also don't forget to update the CL description:
  - you also removed GetInitialXXX()
  - mention PipelineMetadata
  - remove the bit about "There are a few reasonable designs ..." (that message
would be better suited when using Publish+Mail as opposed to being in the CL
description)

[sandersd (OOO until July 31), 2014-03-22 00:40:59]

I've updated the tests as well now. Note that
PipelineTest.DisableAudioRendererDuringInit (pipeline_unittest.cc:648) has
semantically changed as we now report has_audio even when the audio renderer is
disabled.

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc
File media/base/pipeline.cc (right):

https://codereview.chromium.org/206103004/diff/30001/media/base/pipeline.cc#n...
media/base/pipeline.cc:465: PipelineMetadata metadata;
> out of curiosity can you clarify what you meant by "natural ordering"?

I mean that there is no explicit ordering relation between audio and video, so a
quick glance may not spot them being out of order.

[scherkus (not reviewing), 2014-03-22 01:04:05]

lgtm! nice job!

https://codereview.chromium.org/206103004/diff/70001/media/base/pipeline_unit...
File media/base/pipeline_unittest.cc (right):

https://codereview.chromium.org/206103004/diff/70001/media/base/pipeline_unit...
media/base/pipeline_unittest.cc:648: EXPECT_TRUE(metadata_.has_audio);
awesome!

https://codereview.chromium.org/206103004/diff/70001/media/filters/pipeline_i...
File media/filters/pipeline_integration_test_base.cc (right):

https://codereview.chromium.org/206103004/diff/70001/media/filters/pipeline_i...
media/filters/pipeline_integration_test_base.cc:117: base::Closure());
in a follow up CL you should fix this up to make duration_cb non-optional

a popular option if you really don't care about whether its called or not (e.g.,
it's not the focus of the test) is something like the following:

static void DoNothing() {}

...

base::Bind(&DoNothing)

https://codereview.chromium.org/206103004/diff/70001/media/filters/pipeline_i...
media/filters/pipeline_integration_test_base.cc:151: base::Closure());
ditto

[sandersd (OOO until July 31), 2014-03-24 17:46:57]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-03-24 17:48:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sandersd@chromium.org/206103004/70001

[commit-bot: I haz the power, 2014-03-24 21:40:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-24 21:40:49]

Failed to apply patch for content/renderer/media/webmediaplayer_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file content/renderer/media/webmediaplayer_impl.cc
  Hunk #2 FAILED at 965.
  1 out of 5 hunks FAILED -- saving rejects to file
content/renderer/media/webmediaplayer_impl.cc.rej

Patch:       content/renderer/media/webmediaplayer_impl.cc
Index: content/renderer/media/webmediaplayer_impl.cc
diff --git a/content/renderer/media/webmediaplayer_impl.cc
b/content/renderer/media/webmediaplayer_impl.cc
index
b3ea6def86b7322c9f0c55bbc92ea6a0accea502..b76f1e4e22e76eb6e58e728c9089c313cdf2913f
100644
--- a/content/renderer/media/webmediaplayer_impl.cc
+++ b/content/renderer/media/webmediaplayer_impl.cc
@@ -436,19 +436,19 @@ void
WebMediaPlayerImpl::setPreload(WebMediaPlayer::Preload preload) {
 bool WebMediaPlayerImpl::hasVideo() const {
   DCHECK(main_loop_->BelongsToCurrentThread());
 
-  return pipeline_.HasVideo();
+  return pipeline_metadata_.has_video;
 }
 
 bool WebMediaPlayerImpl::hasAudio() const {
   DCHECK(main_loop_->BelongsToCurrentThread());
 
-  return pipeline_.HasAudio();
+  return pipeline_metadata_.has_audio;
 }
 
 blink::WebSize WebMediaPlayerImpl::naturalSize() const {
   DCHECK(main_loop_->BelongsToCurrentThread());
 
-  return blink::WebSize(natural_size_);
+  return blink::WebSize(pipeline_metadata_.natural_size);
 }
 
 bool WebMediaPlayerImpl::paused() const {
@@ -965,33 +965,19 @@ void WebMediaPlayerImpl::OnPipelineError(PipelineStatus
error) {
   InvalidateOnMainThread();
 }
 
-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();
-
-      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;
-    case media::Pipeline::kPrerollCompleted:
-      // Only transition to ReadyStateHaveEnoughData if we don't have
-      // any pending seeks because the transition can cause Blink to
-      // report that the most recent seek has completed.
-      if (!pending_seek_)
-        SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
-      break;
+void WebMediaPlayerImpl::OnPipelineMetadata(
+    media::PipelineMetadata metadata) {
+  DVLOG(1) << "OnPipelineMetadata";
+
+  pipeline_metadata_ = metadata;
+
+  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());
   }
 
   // TODO(scherkus): This should be handled by HTMLMediaElement and controls
@@ -999,6 +985,21 @@ void WebMediaPlayerImpl::OnPipelineBufferingState(
   InvalidateOnMainThread();
 }
 
+void WebMediaPlayerImpl::OnPipelinePrerollCompleted() {
+  DVLOG(1) << "OnPipelinePrerollCompleted";
+
+  // Only transition to ReadyStateHaveEnoughData if we don't have
+  // any pending seeks because the transition can cause Blink to
+  // report that the most recent seek has completed.
+  if (!pending_seek_) {
+    SetReadyState(WebMediaPlayer::ReadyStateHaveEnoughData);
+
+    // TODO(scherkus): This should be handled by HTMLMediaElement and controls
+    // should know when to invalidate themselves http://crbug.com/337015
+    InvalidateOnMainThread();
+  }
+}
+
 void WebMediaPlayerImpl::OnDemuxerOpened() {
   DCHECK(main_loop_->BelongsToCurrentThread());
   client_->mediaSourceOpened(new WebMediaSourceImpl(
@@ -1217,7 +1218,8 @@ void WebMediaPlayerImpl::StartPipeline() {
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineEnded),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineError),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineSeek),
-      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineBufferingState),
+      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelineMetadata),
+      BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnPipelinePrerollCompleted),
       BIND_TO_RENDER_LOOP(&WebMediaPlayerImpl::OnDurationChange));
 }
 
@@ -1279,7 +1281,7 @@ void WebMediaPlayerImpl::OnNaturalSizeChange(gfx::Size
size) {
 
   media_log_->AddEvent(
       media_log_->CreateVideoSizeSetEvent(size.width(), size.height()));
-  natural_size_ = size;
+  pipeline_metadata_.natural_size = size;
 
   client_->sizeChanged();
 }

[sandersd (OOO until July 31), 2014-03-24 22:09:42]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-03-24 22:09:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sandersd@chromium.org/206103004/90001

[commit-bot: I haz the power, 2014-03-25 00:29:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 00:29:22]

Retried try job too often on win_rel for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[sandersd (OOO until July 31), 2014-03-25 01:18:34]

The CQ bit was checked by sandersd@chromium.org

[commit-bot: I haz the power, 2014-03-25 01:19:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sandersd@chromium.org/206103004/110001

[commit-bot: I haz the power, 2014-03-25 05:02:58]

Change committed as 259154

[viettrungluu, 2014-03-25 05:35:14]

On 2014/03/25 05:02:58, I haz the power (commit-bot) wrote:
> Change committed as 259154

Agh, I believe this broke lots of compiles (e.g.,
http://build.chromium.org/p/chromium/builders/Linux/builds/48480/steps/compil...)
-- don't know why the CQ/trybots are happy with it:

FAILED: c++ -MMD -MF obj/media/tools/player_x11/player_x11.player_x11.o.d
-DV8_DEPRECATION_WARNINGS -DBLINK_SCALE_FILTERS_AT_RECORD_TIME
-D_FILE_OFFSET_BITS=64 -DCHROMIUM_BUILD -DTOOLKIT_VIEWS=1
-DUI_COMPOSITOR_IMAGE_TRANSPORT -DUSE_AURA=1 -DUSE_CAIRO=1 -DUSE_GLIB=1
-DUSE_DEFAULT_RENDER_THEME=1 -DUSE_LIBJPEG_TURBO=1 -DUSE_MOJO=1 -DUSE_X11=1
-DUSE_CLIPBOARD_AURAX11=1 -DENABLE_ONE_CLICK_SIGNIN -DUSE_XI2_MT=2
-DENABLE_REMOTING=1 -DENABLE_WEBRTC=1 -DENABLE_PEPPER_CDMS
-DENABLE_CONFIGURATION_POLICY -DENABLE_INPUT_SPEECH -DENABLE_NOTIFICATIONS
-DUSE_UDEV -DENABLE_EGLIMAGE=1 -DENABLE_TASK_MANAGER=1 -DENABLE_EXTENSIONS=1
-DENABLE_PLUGIN_INSTALLATION=1 -DENABLE_PLUGINS=1 -DENABLE_SESSION_SERVICE=1
-DENABLE_THEMES=1 -DENABLE_AUTOFILL_DIALOG=1 -DENABLE_BACKGROUND=1
-DENABLE_AUTOMATION=1 -DENABLE_GOOGLE_NOW=1 -DCLD_VERSION=2
-DENABLE_FULL_PRINTING=1 -DENABLE_PRINTING=1 -DENABLE_SPELLCHECK=1
-DENABLE_CAPTIVE_PORTAL_DETECTION=1 -DENABLE_APP_LIST=1 -DENABLE_SETTINGS_APP=1
-DENABLE_MANAGED_USERS=1 -DENABLE_MDNS=1 -DENABLE_SERVICE_DISCOVERY=1
-DGL_GLEXT_PROTOTYPES -DSK_ENABLE_INST_COUNT=0 -DSK_SUPPORT_GPU=1
'-DGR_GL_CUSTOM_SETUP_HEADER="GrGLConfig_chrome.h"'
-DSK_ENABLE_LEGACY_API_ALIASING=1 -DSK_ATTR_DEPRECATED=SK_NOTHING_ARG1
-DGR_GL_IGNORE_ES3_MSAA=0 -DSK_SUPPORT_LEGACY_LAYERRASTERIZER_API=1
-DSK_WILL_NEVER_DRAW_PERSPECTIVE_TEXT
-DSK_SUPPORT_LEGACY_COMPATIBLEDEVICE_CONFIG=1
-DSK_SUPPORT_LEGACY_PUBLICEFFECTCONSTRUCTORS=1
-DSK_SUPPORT_LEGACY_READPIXELSCONFIG -DSK_SUPPORT_LEGACY_GETCLIPTYPE
-DSK_SUPPORT_LEGACY_GETTOTALCLIP -DSK_SUPPORT_LEGACY_GETTOPDEVICE
-DSK_USE_POSIX_THREADS -DSK_DEFERRED_CANVAS_USES_FACTORIES=1
-DU_USING_ICU_NAMESPACE=0 -DU_STATIC_IMPLEMENTATION -DUSE_NSS=1
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -DNDEBUG -DNVALGRIND
-DDYNAMIC_ANNOTATIONS_ENABLED=0 -D_FORTIFY_SOURCE=2 -I../../third_party/khronos
-I../../gpu -I../.. -I../../skia/config -Igen/ui/gl
-I../../third_party/mesa/src/include -I../../third_party/skia/src/core
-I../../third_party/skia/include/core -I../../third_party/skia/include/effects
-I../../third_party/skia/include/pdf -I../../third_party/skia/include/gpu
-I../../third_party/skia/include/lazy -I../../third_party/skia/include/pathops
-I../../third_party/skia/include/pipe -I../../third_party/skia/include/ports
-I../../third_party/skia/include/utils -I../../skia/ext
-I../../third_party/icu/source/i18n -I../../third_party/icu/source/common
-fstack-protector --param=ssp-buffer-size=4 -Werror -pthread -fno-exceptions
-fno-strict-aliasing -Wall -Wno-unused-parameter -Wno-missing-field-initializers
-fvisibility=hidden -pipe -fPIC -pthread -I/usr/include/glib-2.0
-I/usr/lib/i386-linux-gnu/glib-2.0/include -Wno-unknown-pragmas -msse2
-mfpmath=sse -mmmx -m32 -O2 -fno-ident -fdata-sections -ffunction-sections
-funwind-tables -fno-rtti -fno-threadsafe-statics -fvisibility-inlines-hidden
-Wsign-compare  -c ../../media/tools/player_x11/player_x11.cc -o
obj/media/tools/player_x11/player_x11.player_x11.o
../../media/tools/player_x11/player_x11.cc:96:47: error: variable or field
'OnBufferingState' declared void
../../media/tools/player_x11/player_x11.cc:96:30: error: 'BufferingState' is not
a member of 'media::Pipeline'
../../media/tools/player_x11/player_x11.cc: In function 'void
InitPipeline(media::Pipeline*, const
scoped_refptr<base::SingleThreadTaskRunner>&, media::Demuxer*, const PaintCB&,
bool, base::MessageLoop*)':
../../media/tools/player_x11/player_x11.cc:149:19: error: 'OnBufferingState' was
not declared in this scope
../../media/tools/player_x11/player_x11.cc: In function 'int main(int, char**)':
../../media/tools/player_x11/player_x11.cc:295:17: error: 'class
media::Pipeline' has no member named 'HasVideo'