Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2928123002:
Use the configured remote ssrc instead of relying on the first received packet. (Closed)
|
Created:
3 years, 6 months ago by stefan-webrtc Modified:
3 years, 6 months ago CC:
webrtc-reviews_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, tterriberry_mozilla.com, mflodman Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionUse the configured remote ssrc instead of relying on the first received packet RtpStreamReceiver.
This solves an issue where if the first packet happens to be an RTX packet, it is recovered with an incorrect SSRC.
BUG=webrtc:5654
Review-Url: https://codereview.webrtc.org/2928123002
Cr-Commit-Position: refs/heads/master@{#18512}
Committed: https://chromium.googlesource.com/external/webrtc/+/b4ab381ce372f065b003cac925d10b34d978da71
Patch Set 1 #Patch Set 2 : rebase #
Messages
Total messages: 25 (17 generated)
The CQ bit was checked by stefan@webrtc.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.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/17955)
The CQ bit was checked by stefan@webrtc.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.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/17957)
stefan@webrtc.org changed reviewers: + brandtr@webrtc.org, nisse@webrtc.org
Description was changed from ========== Use the configured remote ssrc instead of relying on the first received packet. BUG= ========== to ========== Use the configured remote ssrc instead of relying on the first received packet. BUG=webrtc:5654 ==========
lgtm. Maybe mention the RtpStreamReceiver in the commit message_
Description was changed from ========== Use the configured remote ssrc instead of relying on the first received packet. BUG=webrtc:5654 ========== to ========== Use the configured remote ssrc instead of relying on the first received packet RtpStreamReceiver. BUG=webrtc:5654 ==========
On 2017/06/09 12:59:56, brandtr wrote: > lgtm. > > Maybe mention the RtpStreamReceiver in the commit message_ Done
lgtm. But I lack a bit of context. Why didn't the code already use the configured ssrc? Is the change a pure cleanup, or does it solve any concrete problem?
On 2017/06/09 13:01:31, nisse-webrtc wrote: > lgtm. > > But I lack a bit of context. Why didn't the code already use the configured > ssrc? Is the change a pure cleanup, or does it solve any concrete problem? I believe it's only legacy, so this is mostly cleanup. It does solve a concrete problem, where an RTX packet is received before any media packets, which currently causes us to recover the media packet inside the RTX packet with an incorrect SSRC.
The CQ bit was checked by stefan@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 stefan@webrtc.org
Description was changed from ========== Use the configured remote ssrc instead of relying on the first received packet RtpStreamReceiver. BUG=webrtc:5654 ========== to ========== Use the configured remote ssrc instead of relying on the first received packet RtpStreamReceiver. This solves an issue where if the first packet happens to be an RTX packet, it is recovered with an incorrect SSRC. BUG=webrtc:5654 ==========
The CQ bit was checked by stefan@webrtc.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1497013444052220,
"parent_rev": "fee994c36717b304731d475ac7eb48b98e8a34e3", "commit_rev":
"b4ab381ce372f065b003cac925d10b34d978da71"}
Message was sent while issue was closed.
Description was changed from ========== Use the configured remote ssrc instead of relying on the first received packet RtpStreamReceiver. This solves an issue where if the first packet happens to be an RTX packet, it is recovered with an incorrect SSRC. BUG=webrtc:5654 ========== to ========== Use the configured remote ssrc instead of relying on the first received packet RtpStreamReceiver. This solves an issue where if the first packet happens to be an RTX packet, it is recovered with an incorrect SSRC. BUG=webrtc:5654 Review-Url: https://codereview.webrtc.org/2928123002 Cr-Commit-Position: refs/heads/master@{#18512} Committed: https://chromium.googlesource.com/external/webrtc/+/b4ab381ce372f065b003cac92... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/webrtc/+/b4ab381ce372f065b003cac92... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
