Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2389703002:
Change DCHECK in VCMDecodedFrameCallback back to just logging. (Closed)
|
Created:
4 years, 2 months ago by sakal Modified:
4 years, 2 months ago Reviewers:
stefan-webrtc CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, zhengzhonghou_agora.io, video-team_agora.io, mflodman, Taylor Brandstetter Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionChange DCHECK in VCMDecodedFrameCallback back to just logging.
This DCHECK is triggered by org.webrtc.PeerConnectionTest#testTrackRemoval.
BUG=webrtc:6465
Committed: https://crrev.com/d227522f573d74ba0d088dfaba7aadbcf4287871
Cr-Commit-Position: refs/heads/master@{#14481}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address comments. #Messages
Total messages: 19 (9 generated)
sakal@webrtc.org changed reviewers: + stefan@webrtc.org
PTAL This DCHECK is being hit here: https://build.chromium.org/p/client.webrtc/builders/Android32%20Tests%20%28L%...
Description was changed from ========== Change DCHECK in VCMDecodedFrameCallback back to just logging. This DCHECK is triggered by org.webrtc.PeerConnectionTest#testTrackRemoval. BUG=None ========== to ========== Change DCHECK in VCMDecodedFrameCallback back to just logging. This DCHECK is triggered by org.webrtc.PeerConnectionTest#testTrackRemoval. BUG=None ==========
Since it is being flakily hit in a test, I am afraid it might happen in real code as well. I would prefer changing the DCHECK to just logging at least for now.
Could you file a bug to investigate / clean this up? And perhaps add a TODO comment referring to the bug. https://codereview.webrtc.org/2389703002/diff/1/webrtc/modules/video_coding/g... File webrtc/modules/video_coding/generic_decoder.cc (right): https://codereview.webrtc.org/2389703002/diff/1/webrtc/modules/video_coding/g... webrtc/modules/video_coding/generic_decoder.cc:79: if (callback) {} https://codereview.webrtc.org/2389703002/diff/1/webrtc/modules/video_coding/g... webrtc/modules/video_coding/generic_decoder.cc:81: else } else {
A TODO could be enough too.
lgtm with the comments addressed.
The CQ bit was checked by sakal@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from stefan@webrtc.org Link to the patchset: https://codereview.webrtc.org/2389703002/#ps20001 (title: "Address comments.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_msan on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_msan/builds/13625)
Description was changed from ========== Change DCHECK in VCMDecodedFrameCallback back to just logging. This DCHECK is triggered by org.webrtc.PeerConnectionTest#testTrackRemoval. BUG=None ========== to ========== Change DCHECK in VCMDecodedFrameCallback back to just logging. This DCHECK is triggered by org.webrtc.PeerConnectionTest#testTrackRemoval. BUG=webrtc:6465 ==========
The CQ bit was checked by sakal@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Message was sent while issue was closed.
Description was changed from ========== Change DCHECK in VCMDecodedFrameCallback back to just logging. This DCHECK is triggered by org.webrtc.PeerConnectionTest#testTrackRemoval. BUG=webrtc:6465 ========== to ========== Change DCHECK in VCMDecodedFrameCallback back to just logging. This DCHECK is triggered by org.webrtc.PeerConnectionTest#testTrackRemoval. BUG=webrtc:6465 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Change DCHECK in VCMDecodedFrameCallback back to just logging. This DCHECK is triggered by org.webrtc.PeerConnectionTest#testTrackRemoval. BUG=webrtc:6465 ========== to ========== Change DCHECK in VCMDecodedFrameCallback back to just logging. This DCHECK is triggered by org.webrtc.PeerConnectionTest#testTrackRemoval. BUG=webrtc:6465 Committed: https://crrev.com/d227522f573d74ba0d088dfaba7aadbcf4287871 Cr-Commit-Position: refs/heads/master@{#14481} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/d227522f573d74ba0d088dfaba7aadbcf4287871 Cr-Commit-Position: refs/heads/master@{#14481} |
