Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2522493002:
Now run EndToEndTest with the WebRTC-NewVideoJitterBuffer experiment. (Closed)
DescriptionNow run EndToEndTest with the WebRTC-NewVideoJitterBuffer experiment.
In this CL:
- EndToEndTests is now parameterized.
- Added VP8 non-rotated unittest.
- CanReceiveUlpfec/CanReceiveFlexFec now use multisets for timestamps.
- pre_decode_image_callback_ is now called before decoding a frame
with the new video jitter buffer.
- Set video rotation when FrameObjects are created.
- Calculate KeyFramesReceivedInPermille in new video jitter buffer.
BUG=webrtc:5514
Committed: https://crrev.com/266f0a44eb27e248fd8bde0115faf672447869b4
Cr-Commit-Position: refs/heads/master@{#15274}
Patch Set 1 #
Total comments: 23
Patch Set 2 : Feedback fixes. #Patch Set 3 : Threading fix. #
Total comments: 2
Patch Set 4 : Feedback fix. #
Total comments: 6
Patch Set 5 : Feedback fixes. #Patch Set 6 : Rebase #
Messages
Total messages: 57 (32 generated)
philipel@webrtc.org changed reviewers: + brandtr@webrtc.org, sprang@webrtc.org
https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_object.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_object.cc:70: VCMPacket* last_packet = packet_buffer_->GetPacket(last_seq_num); Apparently the video rotation is set in the last packet of the frame... https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... File webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc:90: ref_packet_buffer_->InsertPacket(packet); Since an RtpFrameObject now needs information from both the first and the last packet of the frame we now also insert the last packet as well. https://codereview.webrtc.org/2522493002/diff/1/webrtc/video/end_to_end_tests.cc File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:3051: if (GetParam() == "WebRTC-NewVideoJitterBuffer/Enabled/") The reason I haven't fixed this test yet is because I'm not 100% sure what all the metrics means, so I think we might have to discuss how we want the metrics to be measured.
The CQ bit was checked by philipel@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: linux_tsan2 on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_tsan2/builds/17199)
philipel@webrtc.org changed reviewers: + stefan@webrtc.org
https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_buffer2.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_buffer2.cc:370: int key_frames_permille = (static_cast<float>(num_key_frames_) / Are you missing a factor 1000 here? https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_object.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_object.cc:70: VCMPacket* last_packet = packet_buffer_->GetPacket(last_seq_num); On 2016/11/21 15:15:21, philipel wrote: > Apparently the video rotation is set in the last packet of the frame... Is this according to the RFC, or just the way that we do it? https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... File webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc:107: packet.markerBit = seq_num_start == seq_num_end; I think it is nice with parenthesis in situations like this, and below: packet.markerBit = (seq_num_start == seq_num_end); https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/v... File webrtc/modules/video_coding/video_receiver.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/v... webrtc/modules/video_coding/video_receiver.cc:298: if (pre_decode_image_callback_) { Was this a problem found by running the E2E tests? https://codereview.webrtc.org/2522493002/diff/1/webrtc/video/end_to_end_tests.cc File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:764: std::multiset<uint32_t> protected_timestamps_ GUARDED_BY(crit_); Thanks for fixing this. Could you add a short comment why this multiset is needed. (I.e., multi-packet frames where several sequence numbers belong to the same timestamp.)
https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_buffer2.h (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_buffer2.h:44: ~FrameBuffer(); I'd prefer a virtual dtor https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_object.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_object.cc:70: VCMPacket* last_packet = packet_buffer_->GetPacket(last_seq_num); On 2016/11/21 15:15:21, philipel wrote: > Apparently the video rotation is set in the last packet of the frame... Don't we set it on all packets? Or is it that we set it for each frame, even the non-rotated ones? I seem to remember some oddities there. Maybe sync with danilchap@ there. Why do we always say that _rotation_set = true? https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... File webrtc/modules/video_coding/rtp_frame_reference_finder.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... webrtc/modules/video_coding/rtp_frame_reference_finder.cc:54: case kVideoCodecUnknown: Isn't unknown separate from generic? Can we handle it? https://codereview.webrtc.org/2522493002/diff/1/webrtc/video/end_to_end_tests.cc File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:1170: if (GetParam() == "WebRTC-NewVideoJitterBuffer/Enabled/") Can you use constant for this name instead? https://codereview.webrtc.org/2522493002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:3051: if (GetParam() == "WebRTC-NewVideoJitterBuffer/Enabled/") On 2016/11/21 15:15:21, philipel wrote: > The reason I haven't fixed this test yet is because I'm not 100% sure what all > the metrics means, so I think we might have to discuss how we want the metrics > to be measured. This sounds important to get right, so that we can have confidence in the rollout.
https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_buffer2.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_buffer2.cc:370: int key_frames_permille = (static_cast<float>(num_key_frames_) / On 2016/11/21 16:46:45, brandtr wrote: > Are you missing a factor 1000 here? Done. https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_buffer2.h (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_buffer2.h:44: ~FrameBuffer(); On 2016/11/21 16:49:14, språng wrote: > I'd prefer a virtual dtor Why? https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_object.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_object.cc:70: VCMPacket* last_packet = packet_buffer_->GetPacket(last_seq_num); On 2016/11/21 16:46:45, brandtr wrote: > On 2016/11/21 15:15:21, philipel wrote: > > Apparently the video rotation is set in the last packet of the frame... > > Is this according to the RFC, or just the way that we do it? Added (copied from frame_buffer.cc) a comment about it. https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_object.cc:70: VCMPacket* last_packet = packet_buffer_->GetPacket(last_seq_num); On 2016/11/21 16:49:14, språng wrote: > On 2016/11/21 15:15:21, philipel wrote: > > Apparently the video rotation is set in the last packet of the frame... > > Don't we set it on all packets? Or is it that we set it for each frame, even the > non-rotated ones? I seem to remember some oddities there. Maybe sync with > danilchap@ there. > Why do we always say that _rotation_set = true? Added comment, also, this piece of logic is copied from VCMFrameBuffer which always set |_rotation_set| to true when a packet with a marker bit is inserted. https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... File webrtc/modules/video_coding/rtp_frame_reference_finder.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... webrtc/modules/video_coding/rtp_frame_reference_finder.cc:54: case kVideoCodecUnknown: On 2016/11/21 16:49:14, språng wrote: > Isn't unknown separate from generic? Can we handle it? I treat it as generic since that is what is used with the current jitter buffer in the end to end tests. Added a todo to take a look at this. https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... File webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/r... webrtc/modules/video_coding/rtp_frame_reference_finder_unittest.cc:107: packet.markerBit = seq_num_start == seq_num_end; On 2016/11/21 16:46:45, brandtr wrote: > I think it is nice with parenthesis in situations like this, and below: > packet.markerBit = (seq_num_start == seq_num_end); Done. https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/v... File webrtc/modules/video_coding/video_receiver.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/v... webrtc/modules/video_coding/video_receiver.cc:298: if (pre_decode_image_callback_) { On 2016/11/21 16:46:45, brandtr wrote: > Was this a problem found by running the E2E tests? Yes https://codereview.webrtc.org/2522493002/diff/1/webrtc/video/end_to_end_tests.cc File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/video/end_to_end_tests... webrtc/video/end_to_end_tests.cc:764: std::multiset<uint32_t> protected_timestamps_ GUARDED_BY(crit_); On 2016/11/21 16:46:45, brandtr wrote: > Thanks for fixing this. Could you add a short comment why this multiset is > needed. (I.e., multi-packet frames where several sequence numbers belong to the > same timestamp.) Done.
lgtm
The CQ bit was checked by philipel@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 philipel@webrtc.org
The CQ bit was checked by philipel@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: android_more_configs on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/android_more_configs/bu...)
The CQ bit was checked by philipel@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: win_x64_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_dbg/builds/3485)
https://codereview.webrtc.org/2522493002/diff/40001/webrtc/video/end_to_end_t... File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2522493002/diff/40001/webrtc/video/end_to_end_t... webrtc/video/end_to_end_tests.cc:608: protected_timestamps_.erase(header.timestamp); This call will remove all protected timestamps with the given timestamp, defeating the point of the multiset. (See (3) in http://en.cppreference.com/w/cpp/container/multiset/erase.) Please fix (e.g. by replacing with the iterator argument version) before landing :)
https://codereview.webrtc.org/2522493002/diff/40001/webrtc/video/end_to_end_t... File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2522493002/diff/40001/webrtc/video/end_to_end_t... webrtc/video/end_to_end_tests.cc:608: protected_timestamps_.erase(header.timestamp); On 2016/11/24 13:03:57, brandtr wrote: > This call will remove all protected timestamps with the given timestamp, > defeating the point of the multiset. (See (3) in > http://en.cppreference.com/w/cpp/container/multiset/erase.) > > Please fix (e.g. by replacing with the iterator argument version) before landing > :) Done.
The CQ bit was checked by philipel@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: This issue passed the CQ dry run.
Erik, PTAL
ping
Please update the description and include what fixes you had to make, e.g., - Fix rotation by looking at last packet. - Add missing stats. ... https://codereview.webrtc.org/2522493002/diff/60001/webrtc/modules/video_codi... File webrtc/modules/video_coding/rtp_frame_reference_finder.cc (right): https://codereview.webrtc.org/2522493002/diff/60001/webrtc/modules/video_codi... webrtc/modules/video_coding/rtp_frame_reference_finder.cc:54: // Since the EndToEndTests use kVicdeoCodecUnknow we treat it the same as kVideoCodecUnknown ...but I'd prefer to use Generic in the tests instead. Can we do that?
Description was changed from ========== Now run EndToEndTest with the WebRTC-NewVideoJitterBuffer experiment. BUG=webrtc:5514 ========== to ========== Now run EndToEndTest with the WebRTC-NewVideoJitterBuffer experiment. In this CL: - EndToEndTests is now parameterized. - Added VP8 non-rotated unittest. - CanReceiveUlpfec/CanReceiveFlexFec now use multisets for timestamps. - pre_decode_image_callback_ is now called before decoding a frame with the new video jitter buffer. - Set video rotation when FrameObjects are created. - Calculate KeyFramesReceivedInPermille in new video jitter buffer. BUG=webrtc:5514 ==========
https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_buffer2.h (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_buffer2.h:44: ~FrameBuffer(); On 2016/11/22 14:56:23, philipel wrote: > On 2016/11/21 16:49:14, språng wrote: > > I'd prefer a virtual dtor > > Why? Usually good practice, just in case someone subclasses this and has scoped_ptr to it. Can get strangeness deluxe without it and should incur minimal overhead. https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... File webrtc/modules/video_coding/frame_object.cc (right): https://codereview.webrtc.org/2522493002/diff/1/webrtc/modules/video_coding/f... webrtc/modules/video_coding/frame_object.cc:70: VCMPacket* last_packet = packet_buffer_->GetPacket(last_seq_num); On 2016/11/22 14:56:23, philipel wrote: > On 2016/11/21 16:49:14, språng wrote: > > On 2016/11/21 15:15:21, philipel wrote: > > > Apparently the video rotation is set in the last packet of the frame... > > > > Don't we set it on all packets? Or is it that we set it for each frame, even > the > > non-rotated ones? I seem to remember some oddities there. Maybe sync with > > danilchap@ there. > > Why do we always say that _rotation_set = true? > > Added comment, also, this piece of logic is copied from VCMFrameBuffer which > always set |_rotation_set| to true when a packet with a marker bit is inserted. Acknowledged. https://codereview.webrtc.org/2522493002/diff/60001/webrtc/video/end_to_end_t... File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2522493002/diff/60001/webrtc/video/end_to_end_t... webrtc/video/end_to_end_tests.cc:141: "WebRTC-NewVideoJitterBuffer/Disabled/")); Define constants for these names and use below as well? https://codereview.webrtc.org/2522493002/diff/60001/webrtc/video/end_to_end_t... webrtc/video/end_to_end_tests.cc:2714: TEST_P(EndToEndTest, GetStats) { Add the new histogram to this test.
https://codereview.webrtc.org/2522493002/diff/60001/webrtc/modules/video_codi... File webrtc/modules/video_coding/rtp_frame_reference_finder.cc (right): https://codereview.webrtc.org/2522493002/diff/60001/webrtc/modules/video_codi... webrtc/modules/video_coding/rtp_frame_reference_finder.cc:54: // Since the EndToEndTests use kVicdeoCodecUnknow we treat it the same as On 2016/11/28 13:08:52, stefan-webrtc (holmer) wrote: > kVideoCodecUnknown > > ...but I'd prefer to use Generic in the tests instead. Can we do that? The codec type is not set in CallTest or EndToEndTest. I don't know how much work it is to change the default codec from unknown to generic. If we want to change it I think we should do it in a follow-up CL. https://codereview.webrtc.org/2522493002/diff/60001/webrtc/video/end_to_end_t... File webrtc/video/end_to_end_tests.cc (right): https://codereview.webrtc.org/2522493002/diff/60001/webrtc/video/end_to_end_t... webrtc/video/end_to_end_tests.cc:141: "WebRTC-NewVideoJitterBuffer/Disabled/")); On 2016/11/28 13:40:44, språng wrote: > Define constants for these names and use below as well? Done. https://codereview.webrtc.org/2522493002/diff/60001/webrtc/video/end_to_end_t... webrtc/video/end_to_end_tests.cc:2714: TEST_P(EndToEndTest, GetStats) { On 2016/11/28 13:40:44, språng wrote: > Add the new histogram to this test. KeyFramesReceivedInPermille is tested in EndToEndTest::VerifyHistogramStats.
lgtm
The CQ bit was checked by philipel@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from brandtr@webrtc.org Link to the patchset: https://codereview.webrtc.org/2522493002/#ps80001 (title: "Feedback fixes.")
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/12844) ios64_sim_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/12865) ios_arm64_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/15216) ios_arm64_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/15075) ios_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/20432) ios_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/19146)
The CQ bit was checked by philipel@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from brandtr@webrtc.org, stefan@webrtc.org Link to the patchset: https://codereview.webrtc.org/2522493002/#ps100001 (title: "Rebase")
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: mac_asan on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/19758)
The CQ bit was checked by philipel@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": 100001, "attempt_start_ts": 1480351122249730,
"parent_rev": "81a0bf73ccc2f10a1453625f73ad483f1e92a951", "commit_rev":
"fb542d942b2c9e0f5efaa3ffee165f1679be8206"}
Message was sent while issue was closed.
Description was changed from ========== Now run EndToEndTest with the WebRTC-NewVideoJitterBuffer experiment. In this CL: - EndToEndTests is now parameterized. - Added VP8 non-rotated unittest. - CanReceiveUlpfec/CanReceiveFlexFec now use multisets for timestamps. - pre_decode_image_callback_ is now called before decoding a frame with the new video jitter buffer. - Set video rotation when FrameObjects are created. - Calculate KeyFramesReceivedInPermille in new video jitter buffer. BUG=webrtc:5514 ========== to ========== Now run EndToEndTest with the WebRTC-NewVideoJitterBuffer experiment. In this CL: - EndToEndTests is now parameterized. - Added VP8 non-rotated unittest. - CanReceiveUlpfec/CanReceiveFlexFec now use multisets for timestamps. - pre_decode_image_callback_ is now called before decoding a frame with the new video jitter buffer. - Set video rotation when FrameObjects are created. - Calculate KeyFramesReceivedInPermille in new video jitter buffer. BUG=webrtc:5514 ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Now run EndToEndTest with the WebRTC-NewVideoJitterBuffer experiment. In this CL: - EndToEndTests is now parameterized. - Added VP8 non-rotated unittest. - CanReceiveUlpfec/CanReceiveFlexFec now use multisets for timestamps. - pre_decode_image_callback_ is now called before decoding a frame with the new video jitter buffer. - Set video rotation when FrameObjects are created. - Calculate KeyFramesReceivedInPermille in new video jitter buffer. BUG=webrtc:5514 ========== to ========== Now run EndToEndTest with the WebRTC-NewVideoJitterBuffer experiment. In this CL: - EndToEndTests is now parameterized. - Added VP8 non-rotated unittest. - CanReceiveUlpfec/CanReceiveFlexFec now use multisets for timestamps. - pre_decode_image_callback_ is now called before decoding a frame with the new video jitter buffer. - Set video rotation when FrameObjects are created. - Calculate KeyFramesReceivedInPermille in new video jitter buffer. BUG=webrtc:5514 Committed: https://crrev.com/266f0a44eb27e248fd8bde0115faf672447869b4 Cr-Commit-Position: refs/heads/master@{#15274} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/266f0a44eb27e248fd8bde0115faf672447869b4 Cr-Commit-Position: refs/heads/master@{#15274}
Message was sent while issue was closed.
hta@webrtc.org changed reviewers: + hta@webrtc.org
Message was sent while issue was closed.
Commenting on a closed CL: Note that this experiment doubles the runtime of the already slow EndToEnd test suite. Is there a bug filed to remove the looping once the experiment is finished?
Message was sent while issue was closed.
On 2016/11/29 08:23:10, hta-webrtc wrote: > Commenting on a closed CL: > > Note that this experiment doubles the runtime of the already slow EndToEnd test > suite. > > Is there a bug filed to remove the looping once the experiment is finished? Yes, we should track it in a bug so we remember it. Could you file one Philip? Fortunately the runtime doesn't seem too bad. On my machine all tests run within ~20 seconds with and without this patch with gtest-parallel. Of course, without it's 2x slower though... But I think most of our bots run tests in parallel?
Message was sent while issue was closed.
On 2016/11/29 09:14:06, stefan-webrtc (holmer) wrote: > On 2016/11/29 08:23:10, hta-webrtc wrote: > > Commenting on a closed CL: > > > > Note that this experiment doubles the runtime of the already slow EndToEnd > test > > suite. > > > > Is there a bug filed to remove the looping once the experiment is finished? > > Yes, we should track it in a bug so we remember it. Could you file one Philip? > > Fortunately the runtime doesn't seem too bad. On my machine all tests run within > ~20 seconds with and without this patch with gtest-parallel. Of course, without > it's 2x slower though... But I think most of our bots run tests in parallel? Filed a bug for this task: https://bugs.chromium.org/p/webrtc/issues/detail?id=6790 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
