Added debug logging for MSE config changes.

Added debug logging for MSE config changes.


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

[ddorwin, 2013-01-07 19:04:24]

I added these while debugging config change issues and tests. I did not see any
existing logging of when new configs were detected or when an actual change
occurred (other than in one of the decoders).

https://codereview.chromium.org/11777018/diff/1/media/filters/chunk_demuxer.h
File media/filters/chunk_demuxer.h (right):

https://codereview.chromium.org/11777018/diff/1/media/filters/chunk_demuxer.h...
media/filters/chunk_demuxer.h:177: // Callback used to report error strings that
can help the web developer
Copied this from sourc_buffer_stream.h. It's not obvious in the .cc files that
these will all be reported as errors. It would be nice to report info/events
too. (Instead, I had to DVLOG(). I considered renaming this member to
log_error_cb_ to address the .cc issue. I don't think it should increase the
number of lines required, but I didn't want to make a bunch of changes without
buy-in.

[acolwell GONE FROM CHROMIUM, 2013-01-07 19:27:35]

LGTM % nits

https://codereview.chromium.org/11777018/diff/1/media/filters/chunk_demuxer.h
File media/filters/chunk_demuxer.h (right):

https://codereview.chromium.org/11777018/diff/1/media/filters/chunk_demuxer.h...
media/filters/chunk_demuxer.h:177: // Callback used to report error strings that
can help the web developer
On 2013/01/07 19:04:24, ddorwin wrote:
> Copied this from sourc_buffer_stream.h. It's not obvious in the .cc files that
> these will all be reported as errors. It would be nice to report info/events
> too. (Instead, I had to DVLOG(). I considered renaming this member to
> log_error_cb_ to address the .cc issue. I don't think it should increase the
> number of lines required, but I didn't want to make a bunch of changes without
> buy-in.
This isn't intended to only log errors. I don't think we should rename this
especially since scherkus@ had me remove error from the name before checkin. If
the word error in MediaLogEvent::MEDIA_SOURCE_ERROR is troubling you, then I'd
suggest removing that instead of adding error here.

I'm wary of reporting tons of stuff through this mechanism. If our code is
working properly, the web developer doesn't need to know when the config change
is actually processed by the media engine.

https://codereview.chromium.org/11777018/diff/1/media/filters/source_buffer_s...
File media/filters/source_buffer_stream.cc (right):

https://codereview.chromium.org/11777018/diff/1/media/filters/source_buffer_s...
media/filters/source_buffer_stream.cc:932: DVLOG(1) << "Config change (track
buffer ID does not match).";
nit: s/ID/config ID/

https://codereview.chromium.org/11777018/diff/1/media/filters/source_buffer_s...
media/filters/source_buffer_stream.cc:946: DVLOG(1) << "Config change (selected
range ID does not match).";
ditto

[ddorwin, 2013-01-07 22:25:01]

https://codereview.chromium.org/11777018/diff/1/media/filters/source_buffer_s...
File media/filters/source_buffer_stream.cc (right):

https://codereview.chromium.org/11777018/diff/1/media/filters/source_buffer_s...
media/filters/source_buffer_stream.cc:932: DVLOG(1) << "Config change (track
buffer ID does not match).";
On 2013/01/07 19:27:35, acolwell wrote:
> nit: s/ID/config ID/

Done.

https://codereview.chromium.org/11777018/diff/1/media/filters/source_buffer_s...
media/filters/source_buffer_stream.cc:946: DVLOG(1) << "Config change (selected
range ID does not match).";
On 2013/01/07 19:27:35, acolwell wrote:
> ditto

Done.

[commit-bot: I haz the power, 2013-01-07 22:33:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11777018/5001

[commit-bot: I haz the power, 2013-01-08 01:04:44]

Retried try job too often on win_aura for step(s) interactive_ui_tests

[commit-bot: I haz the power, 2013-01-08 01:09:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11777018/5001

[commit-bot: I haz the power, 2013-01-08 18:09:09]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ddorwin@chromium.org/11777018/5001

[commit-bot: I haz the power, 2013-01-08 18:09:30]

Change committed as 175551