Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2627443004:
media: Use different log levels in decoder_stream.cc (Closed)
Descriptionmedia: Use different log levels in decoder_stream.cc
Today we DVLOG(2) exclusively in decoder_stream.cc, making it hard to
filter out unimportant messages and only show important ones.
In this CL, different log levels are used for different events, using
the following rule:
- Level 1: Construction, destruction, initialization and error.
- Level 2: Important repeating events like reset, reinitialization and
config change.
- Level 3: Other more frequent events like read, decode etc.
Review-Url: https://codereview.chromium.org/2627443004
Cr-Commit-Position: refs/heads/master@{#442714}
Committed: https://chromium.googlesource.com/chromium/src/+/74db842f10b264ea4d1d88d541ab5d27de697198
Patch Set 1 #
Total comments: 2
Patch Set 2 : comments addressed #Messages
Total messages: 22 (16 generated)
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
xhwang@chromium.org changed reviewers: + watk@chromium.org
PTAL
tguilbert@chromium.org changed reviewers: + tguilbert@chromium.org
LGTM One drive by comment. https://codereview.chromium.org/2627443004/diff/1/media/filters/decoder_strea... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/2627443004/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:433: FUNCTION_DVLOG(1) << ": Reinitializing decoder after decode error!"; This code path is the one that is related to falling back to a new decoder when we get a decode error without having ever received a valid frame. It is actually selecting a new decoder rather than initializing the current one. WDYT of "Falling back to new decoder after initial decode error!"?
https://codereview.chromium.org/2627443004/diff/1/media/filters/decoder_strea... File media/filters/decoder_stream.cc (right): https://codereview.chromium.org/2627443004/diff/1/media/filters/decoder_strea... media/filters/decoder_stream.cc:433: FUNCTION_DVLOG(1) << ": Reinitializing decoder after decode error!"; On 2017/01/10 19:02:35, tguilbert wrote: > This code path is the one that is related to falling back to a new decoder when > we get a decode error without having ever received a valid frame. It is actually > selecting a new decoder rather than initializing the current one. > > WDYT of "Falling back to new decoder after initial decode error!"? Good catch. Fixed.
comments addressed
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
The CQ bit was checked by xhwang@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by xhwang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tguilbert@chromium.org Link to the patchset: https://codereview.chromium.org/2627443004/#ps20001 (title: "comments addressed")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484087332639090,
"parent_rev": "90824416d3eeae5ec6013b250123df65e9d48032", "commit_rev":
"74db842f10b264ea4d1d88d541ab5d27de697198"}
Message was sent while issue was closed.
Description was changed from
==========
media: Use different log levels in decoder_stream.cc
Today we DVLOG(2) exclusively in decoder_stream.cc, making it hard to
filter out unimportant messages and only show important ones.
In this CL, different log levels are used for different events, using
the following rule:
- Level 1: Construction, destruction, initialization and error.
- Level 2: Important repeating events like reset, reinitialization and
config change.
- Level 3: Other more frequent events like read, decode etc.
==========
to
==========
media: Use different log levels in decoder_stream.cc
Today we DVLOG(2) exclusively in decoder_stream.cc, making it hard to
filter out unimportant messages and only show important ones.
In this CL, different log levels are used for different events, using
the following rule:
- Level 1: Construction, destruction, initialization and error.
- Level 2: Important repeating events like reset, reinitialization and
config change.
- Level 3: Other more frequent events like read, decode etc.
Review-Url: https://codereview.chromium.org/2627443004
Cr-Commit-Position: refs/heads/master@{#442714}
Committed:
https://chromium.googlesource.com/chromium/src/+/74db842f10b264ea4d1d88d541ab...
==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/74db842f10b264ea4d1d88d541ab... |
