New issue
Advanced search Search tips

Issue 722490 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug



Sign in to add a comment

WebRTC receiver ignored PlayoutDelay field

Project Member Reported by sergeyu@chromium.org, May 15 2017

Issue description

Corresponding WebRTC bug: https://bugs.chromium.org/p/webrtc/issues/detail?id=7590#c3

This affects response latency for chromoting when using WebRTC. 

The issue is fixed in trunk. Using this bug to track merge to M59
 
Project Member

Comment 1 by sheriffbot@chromium.org, May 16 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Sergey, have you verified that the fix landed for 7590 actually works?

Comment 3 by sergeyu@google.com, May 17 2017

I've verified that it fixed the issue for chromoting.

Comment 4 by sergeyu@google.com, May 17 2017

Status: Fixed (was: Assigned)
Merged the fix to M59 in https://codereview.chromium.org/2890943002/
Labels: -Merge-Approved-59 Merge-Merged
Project Member

Comment 6 by bugdroid1@chromium.org, May 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69008afaa78dd9d2fc012b3fcb8f2e0a56fc0a1d

commit 69008afaa78dd9d2fc012b3fcb8f2e0a56fc0a1d
Author: sergeyu <sergeyu@chromium.org>
Date: Fri May 19 21:21:27 2017

Ensure that remoting_unittests fail when playout delay > 0.

Added NOTREACHED() WebrtcVideoRendererAdapter for the case when playout
delay > 0. Also updated ConnectionTest_Video to receive 5 tests. With
this change remoting_unittests will fail if playout delay extension
is not sent by the host or is ignored on the receiving side. This will
help to prevent regressions like  crbug.com/webrtc/7590  in the future.
Previously the adapter was only logging a warning in that case.

BUG= 722490 

Review-Url: https://codereview.chromium.org/2887183004
Cr-Commit-Position: refs/heads/master@{#473323}

[modify] https://crrev.com/69008afaa78dd9d2fc012b3fcb8f2e0a56fc0a1d/remoting/protocol/connection_unittest.cc
[modify] https://crrev.com/69008afaa78dd9d2fc012b3fcb8f2e0a56fc0a1d/remoting/protocol/webrtc_video_renderer_adapter.cc

Status: Started (was: Fixed)
Looks like it's still broken when flexfec experiment is enabled

Comment 8 by sergeyu@google.com, May 22 2017

Labels: -Merge-Merged -Hotlist-Merge-Approved Merge-Request-59
Need to merge this WebRTC change to M59: https://codereview.webrtc.org/2899553003
Project Member

Comment 9 by sheriffbot@chromium.org, May 22 2017

Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 10 by bugdroid1@chromium.org, May 22 2017

Labels: merge-merged-59
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/ab7f856fa72a531fa5b9bdafbcc83de21888589e

commit ab7f856fa72a531fa5b9bdafbcc83de21888589e
Author: Sergey Ulanov <sergeyu@chromium.org>
Date: Mon May 22 19:48:25 2017

Update Packet::GetHeader() to copy playout_delay

Packet::GetHeader() wasn't copying playout_delay. As result
playout_delay was ignored when flexfec is enabled.

Patch by Rob McCool <rmccool@google.com>

BUG= webrtc:7590 

Review-Url: https://codereview.webrtc.org/2899553003
Cr-Commit-Position: refs/heads/master@{#18218}
(cherry picked from commit b2c001a79542050d69fa916199f51d5d98626f21)

BUG= chromium:722490 

Review-Url: https://codereview.webrtc.org/2898813002 .
Cr-Commit-Position: refs/branch-heads/59@{#13}
Cr-Branched-From: 10d095d4f743bc16f8e486e156c48a6d023b32c5-refs/heads/master@{#17657}

[modify] https://crrev.com/ab7f856fa72a531fa5b9bdafbcc83de21888589e/webrtc/modules/rtp_rtcp/source/rtp_packet.cc

Status: Fixed (was: Started)
Project Member

Comment 12 by sheriffbot@chromium.org, May 26 2017

Cc: sergeyu@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Approved -Merge-Approved-59

Sign in to add a comment