Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1814023003:
Fixing issues with timestamps in video_quality_test.cc. (Closed)
|
Created:
4 years, 9 months ago by Taylor Brandstetter Modified:
4 years, 9 months ago CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, the sun, pbos-webrtc, perkj_webrtc, mflodman Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionFixing issues with timestamps in video_quality_test.cc.
The fundamental issue is that RTCP packet timestamps were accidentally
being fed into wrap_handler_, causing it to think the 32-bit timestamp
had wrapped around when it actually hadn't.
Was also using a 32-bit timestamp instead of a 64-bit timestamp in one
place, meaning that if wrapping actually DID occur, the test would still
fail due to a 64-bit value being cast to a 32-bit value.
BUG=webrtc:5668
R=pbos@webrtc.org, sprang@webrtc.org
Committed: https://crrev.com/433b95a68585313fb5d607639e6a6f6d3de70427
Cr-Commit-Position: refs/heads/master@{#12055}
Patch Set 1 #Patch Set 2 : Typo. #
Total comments: 1
Messages
Total messages: 18 (8 generated)
deadbeef@webrtc.org changed reviewers: + pbos@webrtc.org
PTAL. This is making tests pretty flaky.
sprang@webrtc.org changed reviewers: + sprang@webrtc.org
Good catch! lgtm
https://codereview.webrtc.org/1814023003/diff/20001/webrtc/video/video_qualit... File webrtc/video/video_quality_test.cc (right): https://codereview.webrtc.org/1814023003/diff/20001/webrtc/video/video_qualit... webrtc/video/video_quality_test.cc:126: Thinking some more about this, perhaps we should even filter on media_type = video. Though there shouldn't be any other type, right now...
The CQ bit was checked by deadbeef@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1814023003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1814023003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: presubmit on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/4264)
lgtm
The CQ bit was checked by deadbeef@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1814023003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1814023003/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
Message was sent while issue was closed.
Description was changed from ========== Fixing issues with timestamps in video_quality_test.cc. The fundamental issue is that RTCP packet timestamps were accidentally being fed into wrap_handler_, causing it to think the 32-bit timestamp had wrapped around when it actually hadn't. Was also using a 32-bit timestamp instead of a 64-bit timestamp in one place, meaning that if wrapping actually DID occur, the test would still fail due to a 64-bit value being cast to a 32-bit value. BUG=webrtc:5668 ========== to ========== Fixing issues with timestamps in video_quality_test.cc. The fundamental issue is that RTCP packet timestamps were accidentally being fed into wrap_handler_, causing it to think the 32-bit timestamp had wrapped around when it actually hadn't. Was also using a 32-bit timestamp instead of a 64-bit timestamp in one place, meaning that if wrapping actually DID occur, the test would still fail due to a 64-bit value being cast to a 32-bit value. BUG=webrtc:5668 R=pbos@webrtc.org, sprang@webrtc.org Committed: https://crrev.com/433b95a68585313fb5d607639e6a6f6d3de70427 Cr-Commit-Position: refs/heads/master@{#12055} ==========
Message was sent while issue was closed.
Description was changed from ========== Fixing issues with timestamps in video_quality_test.cc. The fundamental issue is that RTCP packet timestamps were accidentally being fed into wrap_handler_, causing it to think the 32-bit timestamp had wrapped around when it actually hadn't. Was also using a 32-bit timestamp instead of a 64-bit timestamp in one place, meaning that if wrapping actually DID occur, the test would still fail due to a 64-bit value being cast to a 32-bit value. BUG=webrtc:5668 R=pbos@webrtc.org, sprang@webrtc.org Committed: https://crrev.com/433b95a68585313fb5d607639e6a6f6d3de70427 Cr-Commit-Position: refs/heads/master@{#12055} ========== to ========== Fixing issues with timestamps in video_quality_test.cc. The fundamental issue is that RTCP packet timestamps were accidentally being fed into wrap_handler_, causing it to think the 32-bit timestamp had wrapped around when it actually hadn't. Was also using a 32-bit timestamp instead of a 64-bit timestamp in one place, meaning that if wrapping actually DID occur, the test would still fail due to a 64-bit value being cast to a 32-bit value. BUG=webrtc:5668 R=pbos@webrtc.org, sprang@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/433b95a68585313fb5d607639... ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/433b95a68585313fb5d607639e6a6f6d3de70427 Cr-Commit-Position: refs/heads/master@{#12055}
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as 433b95a68585313fb5d607639e6a6f6d3de70427 (presubmit successful). |
