MediaLog: Log selected audio/video decoder name.

MediaLog: Log selected audio/video decoder name.

The name of selected audio/video decoder will appear in
chrome://media-internals. This is useful for quickly
identifying which decoder is being used (e.g. whether
we are using GpuVideoDecoder for hardware accelerated
video decoding).

BUG=411535
Committed: https://crrev.com/ff459d4eeb89479ae810f6be3d3b5b50fef3324a
Cr-Commit-Position: refs/heads/master@{#293851}

[xhwang, 2014-09-08 17:24:24]

rebase only

[xhwang, 2014-09-08 17:29:47]

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

[xhwang, 2014-09-08 17:31:21]

See https://code.google.com/p/chromium/issues/detail?id=411041#c10 and you'll
understand why I am doing this....

dalecurtis: PTAL
scherkus: FYI

https://codereview.chromium.org/547913002/diff/40001/media/base/media_log.cc
File media/base/media_log.cc (right):

https://codereview.chromium.org/547913002/diff/40001/media/base/media_log.cc#...
media/base/media_log.cc:206: event->params.SetString("INFO", info);
Note we have "error" above. Not sure whether we should go with all lower cases
or all upper cases...

https://codereview.chromium.org/547913002/diff/40001/media/filters/decoder_st...
File media/filters/decoder_stream.cc (right):

https://codereview.chromium.org/547913002/diff/40001/media/filters/decoder_st...
media/filters/decoder_stream.cc:243: MEDIA_LOG(log_cb_) <<
decoder_->GetDisplayName() << " selected";
In chrome://media-internals, you'll see something like

INFO  |  FFmpegVideoDecoder selected

[DaleCurtis, 2014-09-08 17:43:42]

Should this be a player property instead? I ask because long term I'd like the
"Player Properties" to always be cached until closure.

Doing so would allow us to open chrome://media-internals after a player has
already started and see the properties (though not the log events).

[xhwang, 2014-09-08 18:34:44]

On 2014/09/08 17:43:42, DaleCurtis wrote:
> Should this be a player property instead? I ask because long term I'd like the
> "Player Properties" to always be cached until closure.
> 
> Doing so would allow us to open chrome://media-internals after a player has
> already started and see the properties (though not the log events).

Done and PTAL again.

[xhwang, 2014-09-08 18:34:51]

https://codereview.chromium.org/547913002/diff/60001/media/filters/gpu_video_...
File media/filters/gpu_video_decoder.cc (left):

https://codereview.chromium.org/547913002/diff/60001/media/filters/gpu_video_...
media/filters/gpu_video_decoder.cc:190:
media_log_->SetStringProperty("video_decoder", "gpu");
Just realized we actually had something similar. But I feel this CL is still
better since it's more generic and more explicit.

[DaleCurtis, 2014-09-08 18:41:40]

lgtm - thanks for adding this!

https://codereview.chromium.org/547913002/diff/60001/media/filters/decoder_st...
File media/filters/decoder_stream.cc (right):

https://codereview.chromium.org/547913002/diff/60001/media/filters/decoder_st...
media/filters/decoder_stream.cc:242:
media_log_->SetStringProperty("decrypting_demuxer_stream", "yes");
Always set w/ yes/no ?

https://codereview.chromium.org/547913002/diff/60001/media/filters/decoder_st...
media/filters/decoder_stream.cc:249: NOTREACHED();
I take it TEXT type streams never reach this point?

[xhwang, 2014-09-08 20:12:10]

https://codereview.chromium.org/547913002/diff/60001/media/filters/decoder_st...
File media/filters/decoder_stream.cc (right):

https://codereview.chromium.org/547913002/diff/60001/media/filters/decoder_st...
media/filters/decoder_stream.cc:242:
media_log_->SetStringProperty("decrypting_demuxer_stream", "yes");
On 2014/09/08 18:41:39, DaleCurtis wrote:
> Always set w/ yes/no ?

Changed to binary property.

https://codereview.chromium.org/547913002/diff/60001/media/filters/decoder_st...
media/filters/decoder_stream.cc:249: NOTREACHED();
On 2014/09/08 18:41:39, DaleCurtis wrote:
> I take it TEXT type streams never reach this point?

Using ToString() now so the check is moved to compile time.

[xhwang, 2014-09-08 20:57:46]

The CQ bit was checked by xhwang@chromium.org

[commit-bot: I haz the power, 2014-09-08 21:15:00]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/xhwang@chromium.org/547913002/100001

[scherkus (not reviewing), 2014-09-08 23:31:37]

scherkus@chromium.org changed reviewers:
+ scherkus@chromium.org

[scherkus (not reviewing), 2014-09-08 23:31:38]

lgtm -- it'd be nice to think of some way to reduce the amount of
plumbing/boilerplate for adding more logs in the future ... but this is
definitely a great start

[commit-bot: I haz the power, 2014-09-09 04:57:16]

Committed patchset #6 (id:100001) as 7daf5cd4bc580d754367743596dbb2d0e1bf32f7

[commit-bot: I haz the power, 2014-09-10 03:50:26]

Patchset 6 (id:??) landed as
https://crrev.com/ff459d4eeb89479ae810f6be3d3b5b50fef3324a
Cr-Commit-Position: refs/heads/master@{#293851}