Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(33)

Issue 3008043002: Simplify passing video codec factories in media engine (Closed)

Created:
3 years, 3 months ago by magjed_webrtc
Modified:
3 years, 3 months ago
Reviewers:
andersc
CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com, the sun
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Simplify passing video coded factories in media engine We soon want to be able to pass in a new type of video codec factories, see issue 7925 for more information. We currently plumb these video codec factories in a clumsy way from the media engine to the video engine, which will require us to update a lot of places when we add new video codec factory types. This CL cleans up the way we pass in video codec factories to make it easier to add the new factory types. In particular, this CL: * Updates WebRtcVideoEngine to take the video codec factories as arguments in ctor instead of in SetExternalVideoCodec functions. * Remove the Init() function from the vidoe engines - this function is not used. * Update CompositeMediaEngine to take generic variadic arguments, so we can send different arguments for different engines, without having to update this class. * Simplify ownership of video codec factories in WebRtcVideoEngine. WebRtcVideoEngine outlives WebRtcVideoChannel, WebRtcVideoSendStream and WebRtcVideoReceiveStream, so it can keep ownership without having to share ownership with these classes. BUG=webrtc:7925 Review-Url: https://codereview.webrtc.org/3008043002 Cr-Commit-Position: refs/heads/master@{#19794} Committed: https://chromium.googlesource.com/external/webrtc/+/2475ae2e4cdbb7cbfa2a4801cbc36ff7eb73aacd

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+209 lines, -381 lines) Patch
M webrtc/media/base/fakemediaengine.h View 3 chunks +18 lines, -31 lines 0 comments Download
M webrtc/media/base/mediaengine.h View 1 chunk +27 lines, -33 lines 0 comments Download
M webrtc/media/base/videoengine_unittest.h View 2 chunks +3 lines, -2 lines 0 comments Download
M webrtc/media/engine/nullwebrtcvideoengine.h View 1 chunk +2 lines, -18 lines 0 comments Download
D webrtc/media/engine/nullwebrtcvideoengine_unittest.cc View 2 chunks +8 lines, -12 lines 0 comments Download
M webrtc/media/engine/webrtcmediaengine.cc View 1 chunk +17 lines, -49 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine.h View 8 chunks +17 lines, -28 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine.cc View 15 chunks +25 lines, -51 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine_unittest.cc View 37 chunks +92 lines, -157 lines 0 comments Download

Messages

Total messages: 28 (23 generated)
magjed_webrtc
Anders - please take a look.
3 years, 3 months ago (2017-09-11 19:49:43 UTC) #19
andersc
lgtm
3 years, 3 months ago (2017-09-12 10:44:41 UTC) #22
andersc
lgtm
3 years, 3 months ago (2017-09-12 10:44:44 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/3008043002/80001
3 years, 3 months ago (2017-09-12 11:39:33 UTC) #25
commit-bot: I haz the power
3 years, 3 months ago (2017-09-12 11:42:23 UTC) #28
Message was sent while issue was closed.
Committed patchset #1 (id:80001) as
https://chromium.googlesource.com/external/webrtc/+/2475ae2e4cdbb7cbfa2a4801c...

Powered by Google App Engine
This is Rietveld 408576698