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

Issue 3007073002: Add new video codec factories (Closed)

Created:
2 months, 2 weeks ago by magjed_webrtc
Modified:
2 months, 1 week 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:

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 ...
2 months, 2 weeks ago (2017-09-05 15:56:30 UTC) #9
magjed_webrtc
Anders - please take a look.
2 months, 2 weeks 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 ...
2 months, 2 weeks 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 ...
2 months, 2 weeks 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: ...
2 months, 1 week 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: ...
2 months, 1 week 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 ...
2 months, 1 week ago (2017-09-12 12:08:48 UTC) #30
andersc
neat! lgtm
2 months, 1 week 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 ...
2 months, 1 week 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 ...
2 months, 1 week 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
2 months, 1 week 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; ...
2 months, 1 week 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
2 months, 1 week ago (2017-09-13 17:21:44 UTC) #42
commit-bot: I haz the power
Failed to commit the patch.
2 months, 1 week ago (2017-09-13 18:30:08 UTC) #47
magjed_webrtc
2 months, 1 week 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).

Powered by Google App Engine
This is Rietveld efc10ee0f