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

Issue 3007073002: Add new video codec factories (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks, 1 day ago by magjed_webrtc
Modified:
5 days, 3 hours ago
Reviewers:
andersc, stefan-webrtc
CC:
webrtc-reviews_webrtc.org, the sun, tterriberry_mozilla.com, kwiberg-webrtc, brandtr
Target Ref:
refs/heads/master
Project:
webrtc
Visibility:
Public.

Description

Add new video codec factories This CL adds interfaces for the new video codec factories and wires them up in WebRtcVideoEngine. The default behavior is unmodified however, and the new code is currently unused except for the tests. A follow-up CL will be uploaded for exposing them in the PeerConnectionFactory API: https://codereview.webrtc.org/3004353002/. BUG=webrtc:7925 R=andersc@webrtc.org, stefan@webrtc.org Review-Url: https://codereview.webrtc.org/3007073002 . Cr-Commit-Position: refs/heads/master@{#19828} Committed: https://webrtc.googlesource.com/src/+/d4b0c0562323e24d3075b6831bafa62ea8bf32bd

Patch Set 1 #

Patch Set 2 : Add tests #

Total comments: 11

Patch Set 3 : Rebase SdpVideoFormat #

Total comments: 2

Patch Set 4 : Remove legacy comment. #

Patch Set 5 : . #

Patch Set 6 : . #

Unified diffs Side-by-side diffs Delta from patch set Stats (+332 lines, -5 lines) Patch
M webrtc/api/video_codecs/BUILD.gn View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
A webrtc/api/video_codecs/video_decoder_factory.h View 1 2 1 chunk +39 lines, -0 lines 0 comments Download
A webrtc/api/video_codecs/video_encoder_factory.h View 1 2 1 chunk +57 lines, -0 lines 0 comments Download
M webrtc/media/engine/webrtcvideodecoderfactory.h View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M webrtc/media/engine/webrtcvideoencoderfactory.h View 1 2 3 2 chunks +3 lines, -3 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine.h View 1 2 3 2 chunks +9 lines, -0 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine.cc View 1 2 4 chunks +75 lines, -2 lines 0 comments Download
M webrtc/media/engine/webrtcvideoengine_unittest.cc View 1 2 4 chunks +145 lines, -0 lines 0 comments Download
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 49 (34 generated)
magjed_webrtc
Stefan - please review webrtc/api/video_codecs. There is a design doc in https://docs.google.com/a/google.com/document/d/12AhwdOP6hGauggUVo-wReuFaJhTdpaVIBdJzupqkzjM/edit?usp=sharing if you want ...
1 week, 6 days ago (2017-09-05 15:56:30 UTC) #9
magjed_webrtc
Anders - please take a look.
1 week, 6 days ago (2017-09-05 15:57:26 UTC) #11
stefan-webrtc
https://codereview.webrtc.org/3007073002/diff/40001/webrtc/api/video_codecs/video_decoder_factory.h File webrtc/api/video_codecs/video_decoder_factory.h (right): https://codereview.webrtc.org/3007073002/diff/40001/webrtc/api/video_codecs/video_decoder_factory.h#newcode19 webrtc/api/video_codecs/video_decoder_factory.h:19: } // namespace cricket What do you think of ...
1 week, 5 days ago (2017-09-06 12:40:42 UTC) #12
andersc
https://codereview.webrtc.org/3007073002/diff/40001/webrtc/api/video_codecs/video_decoder_factory.h File webrtc/api/video_codecs/video_decoder_factory.h (right): https://codereview.webrtc.org/3007073002/diff/40001/webrtc/api/video_codecs/video_decoder_factory.h#newcode34 webrtc/api/video_codecs/video_decoder_factory.h:34: virtual std::unique_ptr<VideoDecoder> CreateVideoDecoder( Do we also need a version ...
1 week, 5 days ago (2017-09-06 15:00:06 UTC) #13
magjed_webrtc
https://codereview.webrtc.org/3007073002/diff/40001/webrtc/api/video_codecs/video_decoder_factory.h File webrtc/api/video_codecs/video_decoder_factory.h (right): https://codereview.webrtc.org/3007073002/diff/40001/webrtc/api/video_codecs/video_decoder_factory.h#newcode19 webrtc/api/video_codecs/video_decoder_factory.h:19: } // namespace cricket On 2017/09/06 12:40:42, stefan-webrtc wrote: ...
1 week, 1 day ago (2017-09-10 15:27:50 UTC) #19
kwiberg-webrtc
https://codereview.webrtc.org/3007073002/diff/40001/webrtc/api/video_codecs/video_decoder_factory.h File webrtc/api/video_codecs/video_decoder_factory.h (right): https://codereview.webrtc.org/3007073002/diff/40001/webrtc/api/video_codecs/video_decoder_factory.h#newcode19 webrtc/api/video_codecs/video_decoder_factory.h:19: } // namespace cricket On 2017/09/10 15:27:49, magjed_webrtc wrote: ...
1 week, 1 day ago (2017-09-10 19:00:21 UTC) #20
magjed_webrtc
Ping - please take a look again. All dependencies have been landed now. https://codereview.webrtc.org/3007073002/diff/40001/webrtc/api/video_codecs/video_decoder_factory.h File ...
6 days, 23 hours ago (2017-09-12 12:08:48 UTC) #30
andersc
neat! lgtm
6 days, 22 hours ago (2017-09-12 13:11:16 UTC) #33
stefan-webrtc
One nit, otherwise lgtm https://codereview.webrtc.org/3007073002/diff/120001/webrtc/api/video_codecs/video_encoder_factory.h File webrtc/api/video_codecs/video_encoder_factory.h (right): https://codereview.webrtc.org/3007073002/diff/120001/webrtc/api/video_codecs/video_encoder_factory.h#newcode35 webrtc/api/video_codecs/video_encoder_factory.h:35: // webrtc::ViEExternalCodec::RegisterExternalSendCodec. This no longer ...
5 days, 23 hours ago (2017-09-13 12:39:18 UTC) #34
magjed_webrtc
https://codereview.webrtc.org/3007073002/diff/120001/webrtc/api/video_codecs/video_encoder_factory.h File webrtc/api/video_codecs/video_encoder_factory.h (right): https://codereview.webrtc.org/3007073002/diff/120001/webrtc/api/video_codecs/video_encoder_factory.h#newcode35 webrtc/api/video_codecs/video_encoder_factory.h:35: // webrtc::ViEExternalCodec::RegisterExternalSendCodec. On 2017/09/13 12:39:18, stefan-webrtc wrote: > This ...
5 days, 22 hours ago (2017-09-13 12:53:50 UTC) #35
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/3007073002/140001
5 days, 22 hours ago (2017-09-13 12:54:15 UTC) #38
commit-bot: I haz the power
Try jobs failed on following builders: linux_compile_dbg on master.tryserver.webrtc (JOB_TIMED_OUT, build has not started yet; ...
5 days, 20 hours ago (2017-09-13 14:54:28 UTC) #40
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/3007073002/140001
5 days, 18 hours ago (2017-09-13 17:21:44 UTC) #42
commit-bot: I haz the power
Failed to commit the patch.
5 days, 17 hours ago (2017-09-13 18:30:08 UTC) #47
magjed_webrtc
5 days, 3 hours ago (2017-09-14 08:25:04 UTC) #49
Message was sent while issue was closed.
Committed patchset #6 (id:180001) manually as
d4b0c0562323e24d3075b6831bafa62ea8bf32bd (presubmit successful).
Sign in to reply to this message.

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