Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1957963002:
Allow WebRTC to offer receiving capability for 120ms Opus packets. (Closed)
|
Created:
4 years, 7 months ago by minyue-webrtc Modified:
4 years, 7 months ago Reviewers:
the sun CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAllow WebRTC to offer receiving capability for 120ms Opus packets.
TEST=Build Chromium for receiving + a special AppRTCDemo built with 120ms Opus sending capability. Call went well.
BUG=
R=solenberg@webrtc.org
Committed: https://chromium.googlesource.com/external/webrtc/+/b031a2e8629f6a9c9ed6b4d2337139cc6e484cc6
Patch Set 1 #Patch Set 2 : fixing unittest #
Total comments: 1
Messages
Total messages: 19 (8 generated)
Description was changed from ========== Allow WebRTC to offer receiving capability for 120ms Opus packets. BUG= ========== to ========== Allow WebRTC to offer receiving capability for 120ms Opus packets. TEST=Build Chromium for receiving + a special AppRTCDemo built with 120ms Opus sending capability. Call went well. BUG= ==========
minyue@webrtc.org changed reviewers: + solenberg@webrtc.org
Hi Fredrik, Here is a small CL for you to review. Thanks in advance!
lgtm
The CQ bit was checked by minyue@webrtc.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1957963002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1957963002/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_ubsan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan/builds/1878)
The CQ bit was checked by minyue@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1957963002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1957963002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_compile_x86_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x86_dbg...)
Hi Fredrik, Sorry for not realizing this earlier, but a unit test needs to be updated. It is known that the pmaxtime will not be added if it equals the default value (120). See the new patch set. Thanks!
https://codereview.webrtc.org/1957963002/diff/20001/webrtc/media/engine/webrt... File webrtc/media/engine/webrtcvoiceengine_unittest.cc (left): https://codereview.webrtc.org/1957963002/diff/20001/webrtc/media/engine/webrt... webrtc/media/engine/webrtcvoiceengine_unittest.cc:3614: EXPECT_EQ("60", it->params.find("maxptime")->second); Did you mean update the unit test by removing? Is it possible to change the constant to "120" instead?
On 2016/05/10 13:20:57, the sun wrote: > https://codereview.webrtc.org/1957963002/diff/20001/webrtc/media/engine/webrt... > File webrtc/media/engine/webrtcvoiceengine_unittest.cc (left): > > https://codereview.webrtc.org/1957963002/diff/20001/webrtc/media/engine/webrt... > webrtc/media/engine/webrtcvoiceengine_unittest.cc:3614: EXPECT_EQ("60", > it->params.find("maxptime")->second); > Did you mean update the unit test by removing? Is it possible to change the > constant to "120" instead? Still lgtm
On 2016/05/10 13:20:57, the sun wrote: > https://codereview.webrtc.org/1957963002/diff/20001/webrtc/media/engine/webrt... > File webrtc/media/engine/webrtcvoiceengine_unittest.cc (left): > > https://codereview.webrtc.org/1957963002/diff/20001/webrtc/media/engine/webrt... > webrtc/media/engine/webrtcvoiceengine_unittest.cc:3614: EXPECT_EQ("60", > it->params.find("maxptime")->second); > Did you mean update the unit test by removing? Is it possible to change the > constant to "120" instead? reading from webrtcvoiceengine.cc line 288, if the maxptime equals default, it will not be added to the codec params.
Description was changed from ========== Allow WebRTC to offer receiving capability for 120ms Opus packets. TEST=Build Chromium for receiving + a special AppRTCDemo built with 120ms Opus sending capability. Call went well. BUG= ========== to ========== Allow WebRTC to offer receiving capability for 120ms Opus packets. TEST=Build Chromium for receiving + a special AppRTCDemo built with 120ms Opus sending capability. Call went well. BUG= R=solenberg@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/b031a2e8629f6a9c9ed6b4d23... ==========
Message was sent while issue was closed.
Description was changed from ========== Allow WebRTC to offer receiving capability for 120ms Opus packets. TEST=Build Chromium for receiving + a special AppRTCDemo built with 120ms Opus sending capability. Call went well. BUG= R=solenberg@webrtc.org Committed: https://chromium.googlesource.com/external/webrtc/+/b031a2e8629f6a9c9ed6b4d23... ========== to ========== Allow WebRTC to offer receiving capability for 120ms Opus packets. TEST=Build Chromium for receiving + a special AppRTCDemo built with 120ms Opus sending capability. Call went well. BUG= R=solenberg@webrtc.org Committed: https://crrev.com/b031a2e8629f6a9c9ed6b4d2337139cc6e484cc6 Cr-Commit-Position: refs/heads/master@{#12673} ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) manually as b031a2e8629f6a9c9ed6b4d2337139cc6e484cc6 (presubmit successful). |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
