Request a GpuVideoAcceleratorFactories when constructing decoders.

Request a GpuVideoAcceleratorFactories when constructing decoders.

This simple change allows us to continue to create working hardware
decoders after a context lost while playback is suspended. (Which is
of particular interest because this can happen while backgrounded on
Android.)

Details: Instead of passing a GpuVideoAcceleratorFactories to the
DefaultRendererFactory, and thus always using the same one for a given
media player instance, pass a getter callback instead. Now
DefaultRendererFactory can get the latest GpuVideoAcceleratorFactories
instance each time a VideoDecoder is constructed.

BUG=580386
Committed: https://crrev.com/0c2f582f1c68259ea17d2e95141fa0452ca7ceb4
Cr-Commit-Position: refs/heads/master@{#381044}

[sandersd (OOO until July 31), 2016-03-11 22:28:30]

sandersd@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[DaleCurtis, 2016-03-11 23:46:19]

dalecurtis@chromium.org changed reviewers:
+ dcastagna@chromium.org

[DaleCurtis, 2016-03-11 23:46:19]

dalecurtis@chromium.org changed reviewers:
+ dcastagna@chromium.org

[DaleCurtis, 2016-03-11 23:46:20]

lgtm, but +dcastagna@ in case I'm missing something here.

https://codereview.chromium.org/1784173005/diff/20001/media/renderers/default...
File media/renderers/default_renderer_factory.h (right):

https://codereview.chromium.org/1784173005/diff/20001/media/renderers/default...
media/renderers/default_renderer_factory.h:31:
base::Callback<GpuVideoAcceleratorFactories*()> get_gpu_factories_cb,
const&?

https://codereview.chromium.org/1784173005/diff/20001/media/renderers/default...
media/renderers/default_renderer_factory.h:57:
base::Callback<GpuVideoAcceleratorFactories*()> get_gpu_factories_cb_;
Create typedef/using entry for the callback type.

[DaleCurtis, 2016-03-11 23:46:20]

lgtm, but +dcastagna@ in case I'm missing something here.

https://codereview.chromium.org/1784173005/diff/20001/media/renderers/default...
File media/renderers/default_renderer_factory.h (right):

https://codereview.chromium.org/1784173005/diff/20001/media/renderers/default...
media/renderers/default_renderer_factory.h:31:
base::Callback<GpuVideoAcceleratorFactories*()> get_gpu_factories_cb,
const&?

https://codereview.chromium.org/1784173005/diff/20001/media/renderers/default...
media/renderers/default_renderer_factory.h:57:
base::Callback<GpuVideoAcceleratorFactories*()> get_gpu_factories_cb_;
Create typedef/using entry for the callback type.

[sandersd (OOO until July 31), 2016-03-11 23:56:22]

https://codereview.chromium.org/1784173005/diff/20001/media/renderers/default...
File media/renderers/default_renderer_factory.h (right):

https://codereview.chromium.org/1784173005/diff/20001/media/renderers/default...
media/renderers/default_renderer_factory.h:31:
base::Callback<GpuVideoAcceleratorFactories*()> get_gpu_factories_cb,
On 2016/03/11 23:46:20, DaleCurtis wrote:
> const&?

Done.

https://codereview.chromium.org/1784173005/diff/20001/media/renderers/default...
media/renderers/default_renderer_factory.h:57:
base::Callback<GpuVideoAcceleratorFactories*()> get_gpu_factories_cb_;
On 2016/03/11 23:46:20, DaleCurtis wrote:
> Create typedef/using entry for the callback type.

Done.

[sandersd (OOO until July 31), 2016-03-11 23:56:36]

sandersd@chromium.org changed reviewers:
+ jochen@chromium.org

[sandersd (OOO until July 31), 2016-03-11 23:56:37]

jochen@chromium.org: Please review changes in content/renderer

[sandersd (OOO until July 31), 2016-03-12 00:00:04]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-12 00:00:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1784173005/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1784173005/40001

[commit-bot: I haz the power, 2016-03-12 00:14:58]

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

[commit-bot: I haz the power, 2016-03-12 00:14:58]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[sandersd (OOO until July 31), 2016-03-12 00:27:04]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-12 00:28:14]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1784173005/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1784173005/60001

[commit-bot: I haz the power, 2016-03-12 00:50:32]

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

[commit-bot: I haz the power, 2016-03-12 00:50:33]

Dry run: Try jobs failed on following builders:
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sandersd (OOO until July 31), 2016-03-12 00:52:31]

The CQ bit was checked by sandersd@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-12 00:52:59]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1784173005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1784173005/80001

[commit-bot: I haz the power, 2016-03-12 04:39:21]

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

[commit-bot: I haz the power, 2016-03-12 04:39:22]

Dry run: This issue passed the CQ dry run.

[Daniele Castagna, 2016-03-13 23:40:55]

The patch LGTM.

I think the alias GetGpuFactoriesCB =
base::Callback<GpuVideoAcceleratorFactories*()>; can be a little bit misleading
though.

When it's used from the outside (i.e:
DefaultRendererFactory::GetGpuFactoriewCB()) it seems like you're calling a
static method that returns a valid callback, while you're actually creating an
empty callback.

[jochen (gone - plz use gerrit), 2016-03-14 14:24:45]

lgtm

[sandersd (OOO until July 31), 2016-03-14 18:25:42]

On 2016/03/13 23:40:55, Daniele Castagna wrote:
> The patch LGTM.
> 
> I think the alias GetGpuFactoriesCB =
> base::Callback<GpuVideoAcceleratorFactories*()>; can be a little bit
misleading
> though.
> 
> When it's used from the outside (i.e:
> DefaultRendererFactory::GetGpuFactoriewCB()) it seems like you're calling a
> static method that returns a valid callback, while you're actually creating an
> empty callback.

Agreed, but I can't find a better solution (that's consistent with how things
are done elsewhere in media/). At least within media/, we're used to CB being a
type, so that does limit the amount of confusion. At least it's not used in
production code.

[sandersd (OOO until July 31), 2016-03-14 18:25:50]

The CQ bit was checked by sandersd@chromium.org

[sandersd (OOO until July 31), 2016-03-14 18:25:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org
Link to the patchset: https://codereview.chromium.org/1784173005/#ps80001
(title: "Update test_mojo_media_client.cc")

[commit-bot: I haz the power, 2016-03-14 18:26:08]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1784173005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1784173005/80001

[commit-bot: I haz the power, 2016-03-14 19:39:56]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-03-14 19:41:09]

Description was changed from

==========
Request a GpuVideoAcceleratorFactories when constructing decoders.

This simple change allows us to continue to create working hardware
decoders after a context lost while playback is suspended. (Which is
of particular interest because this can happen while backgrounded on
Android.)

Details: Instead of passing a GpuVideoAcceleratorFactories to the
DefaultRendererFactory, and thus always using the same one for a given
media player instance, pass a getter callback instead. Now
DefaultRendererFactory can get the latest GpuVideoAcceleratorFactories
instance each time a VideoDecoder is constructed.

BUG=580386
==========

to

==========
Request a GpuVideoAcceleratorFactories when constructing decoders.

This simple change allows us to continue to create working hardware
decoders after a context lost while playback is suspended. (Which is
of particular interest because this can happen while backgrounded on
Android.)

Details: Instead of passing a GpuVideoAcceleratorFactories to the
DefaultRendererFactory, and thus always using the same one for a given
media player instance, pass a getter callback instead. Now
DefaultRendererFactory can get the latest GpuVideoAcceleratorFactories
instance each time a VideoDecoder is constructed.

BUG=580386

Committed: https://crrev.com/0c2f582f1c68259ea17d2e95141fa0452ca7ceb4
Cr-Commit-Position: refs/heads/master@{#381044}
==========

[commit-bot: I haz the power, 2016-03-14 19:41:10]

Patchset 5 (id:??) landed as
https://crrev.com/0c2f582f1c68259ea17d2e95141fa0452ca7ceb4
Cr-Commit-Position: refs/heads/master@{#381044}