Chromium Code Reviews
Help | Chromium Project | Sign in
(1)

Issue 2826263004: Move responsibility for RTP header extensions on video receive.

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 days, 17 hours ago by nisse-webrtc
Modified:
5 days, 17 hours ago
CC:
webrtc-reviews_webrtc.org, interface-changes_webrtc.org, video-team_agora.io, yujie_mao (webrtc), zhengzhonghou_agora.io, stefan-webrtc, tterriberry_mozilla.com, qiang.lu, niklas.enbom, peah-webrtc, the sun, mflodman
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Move responsibility for RTP header extensions on video receive. Delete VideoReceiveStream::Config::Rtp::extensions and FlexfecReceiveStream::Config::rtp_header_extensions. Instead, let WebRtcSession pass the list of extensions to Call, which is responsible for the parsing. BUG=webrtc:7135

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+149 lines, -67 lines) Patch
M webrtc/call/bitrate_estimator_tests.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M webrtc/call/call.h View 1 chunk +5 lines, -0 lines 0 comments Download
M webrtc/call/call.cc View 12 chunks +34 lines, -20 lines 0 comments Download
M webrtc/call/flexfec_receive_stream.h View 1 chunk +0 lines, -3 lines 0 comments Download
M webrtc/call/flexfec_receive_stream_impl.cc View 1 chunk +1 line, -7 lines 0 comments Download
M webrtc/call/flexfec_receive_stream_unittest.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/call/rampup_tests.cc View 2 chunks +7 lines, -1 line 0 comments Download
M webrtc/logging/rtc_event_log/rtc_event_log.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/logging/rtc_event_log/rtc_event_log_parser.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/logging/rtc_event_log/rtc_event_log_unittest.cc View 2 chunks +2 lines, -0 lines 0 comments Download
M webrtc/logging/rtc_event_log/rtc_event_log_unittest_helper.cc View 4 chunks +4 lines, -0 lines 0 comments Download
M webrtc/media/base/mediachannel.h View 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/media/engine/fakewebrtccall.h View 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/media/engine/fakewebrtccall.cc View 1 chunk +3 lines, -0 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine2.cc View 3 chunks +0 lines, -8 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine2_unittest.cc View 15 chunks +24 lines, -14 lines 0 comments Download
M webrtc/pc/fakemediacontroller.h View 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/pc/mediacontroller.h View 2 chunks +5 lines, -0 lines 0 comments Download
M webrtc/pc/mediacontroller.cc View 1 chunk +14 lines, -0 lines 0 comments Download
M webrtc/pc/webrtcsession.cc View 1 chunk +10 lines, -0 lines 0 comments Download
M webrtc/test/call_test.cc View 2 chunks +4 lines, -0 lines 0 comments Download
M webrtc/tools/event_log_visualizer/analyzer.cc View 1 chunk +4 lines, -0 lines 0 comments Download
M webrtc/video/end_to_end_tests.cc View 2 chunks +4 lines, -1 line 0 comments Download
M webrtc/video/rtp_stream_receiver.h View 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/video/rtp_stream_receiver.cc View 3 chunks +4 lines, -1 line 0 comments Download
M webrtc/video/video_quality_test.cc View 3 chunks +4 lines, -0 lines 0 comments Download
M webrtc/video/video_receive_stream.cc View 1 chunk +1 line, -9 lines 0 comments Download
M webrtc/video/video_send_stream_tests.cc View 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/video_receive_stream.h View 1 chunk +0 lines, -3 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 2 (1 generated)
nisse-webrtc
5 days, 17 hours ago (2017-04-21 12:14:37 UTC) #2
This is a somewhat experimental cl, attempting to move responsibility for
received rtp header extensions from video receive streams to Call, with the
intention of later moving it one step more, to RtpTransportController. The idea
is that the extensions map really is a transport-level property, not a stream
property. 

I've also been considering the alternative of trying to delete
IdentifyExtensions, and not have any central ownership of the extensions map;
instead each piece of code interesting in some extension would need to know its
configured numeric id, and use RtpPacket::GetRawExtension to access it.

One effect is that receive streams are no longer destroyed and recreated if
header extensions are renegotiated mid-stream, which I guess is an improvement.

Lots of unit tests are broken, I don't expect any fundamental difficulties in
fixing them.

Also rtc eventlogs are broken, since they depend on the model that header
extensions are associated with a stream rather than a transport. Probably needs
more care to get right.

I'd also appreciate advice on which order to do things in. This is related to
https://codereview.webrtc.org/2709723003/. I think it is easiest to land the
latter cl first; then the method to set extensions would naturally belong to the
rtp demuxer, one drawback that we probably have to do the corresonding audio
changes in the same cl.

It's also related to my cl https://codereview.webrtc.org/2794943002/, which
atttempts to delete MediaController. If that is landed first, the WebRtcSession
--> Call signaling in this cl wouldn't need the ugly detour via MediaController.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46