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

Issue 3007073002: Add new video codec factories (Closed)

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