Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2334113004:
Do not update stream synchronization if no new video packet has been received since last u… (Closed)
|
Created:
4 years, 3 months ago by åsapersson Modified:
4 years, 3 months ago CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, perkj_webrtc, mflodman Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionDo not update stream synchronization if no new video packet has been received since last update (e.g. video muted).
BUG=
Committed: https://crrev.com/b0c1b4e24d5250f3d9def9ccc3300e9a1237953f
Cr-Commit-Position: refs/heads/master@{#14271}
Patch Set 1 #
Total comments: 1
Patch Set 2 #Messages
Total messages: 19 (9 generated)
Description was changed from ========== Do not update stream synchronization if no new audio or video packet has been received since last update (e.g. video muted). BUG= ========== to ========== Do not update stream synchronization if no new audio or video packet has been received since last update (e.g. video muted). BUG= ==========
asapersson@webrtc.org changed reviewers: + mflodman@webrtc.org, stefan@webrtc.org
Patchset #1 (id:1) has been deleted
https://codereview.webrtc.org/2334113004/diff/20001/webrtc/video/rtp_streams_... File webrtc/video/rtp_streams_synchronizer.cc (right): https://codereview.webrtc.org/2334113004/diff/20001/webrtc/video/rtp_streams_... webrtc/video/rtp_streams_synchronizer.cc:173: bool RtpStreamsSynchronizer::GetStreamSyncOffsetInMs( Will this method also cause stats problems if it's being called when there are no new frames? Mainly for the opposite problem I assume, when audio is muted and video is received.
Can we add tests for this as well? Would be great to verify that we can actually trigger the problem. Either a unittest or add it to the call perf tests where we already test sync today.
Description was changed from ========== Do not update stream synchronization if no new audio or video packet has been received since last update (e.g. video muted). BUG= ========== to ========== Do not update stream synchronization if no new video packet has been received since last update (e.g. video muted). BUG= ==========
lgtm
The CQ bit was checked by asapersson@webrtc.org
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: ios32_sim_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/10666)
The CQ bit was checked by asapersson@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 ========== Do not update stream synchronization if no new video packet has been received since last update (e.g. video muted). BUG= ========== to ========== Do not update stream synchronization if no new video packet has been received since last update (e.g. video muted). BUG= ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Do not update stream synchronization if no new video packet has been received since last update (e.g. video muted). BUG= ========== to ========== Do not update stream synchronization if no new video packet has been received since last update (e.g. video muted). BUG= Committed: https://crrev.com/b0c1b4e24d5250f3d9def9ccc3300e9a1237953f Cr-Commit-Position: refs/heads/master@{#14271} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/b0c1b4e24d5250f3d9def9ccc3300e9a1237953f Cr-Commit-Position: refs/heads/master@{#14271}
Message was sent while issue was closed.
On 2016/09/17 08:00:13, commit-bot: I haz the power wrote: > Patchset 2 (id:??) landed as > https://crrev.com/b0c1b4e24d5250f3d9def9ccc3300e9a1237953f > Cr-Commit-Position: refs/heads/master@{#14271} per stefan's comment: is this unit-testable? Want more confidence before CPing this. |
