Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
3006713002:
WebRtcVideoEngine: Encapsulate logic for unifying internal and external video codecs (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. |
DescriptionWebRtcVideoEngine: Encapsulate logic for unifying internal and external video codecs
This CL encapsulates the logic for unifying the internal and external
video encoders into a helper class. The purpose is to prepare for
introducing a new video encoder factory interface that inherently
represents all encoders (i.e. both internal and external). A helper
interface EncoderFactoryAdapter is introduced that both the old
WebRtcVideoEncoderFactory and the new factory interface can implement
and serves as common point to leave the rest of the code unchanged.
BUG=webrtc:7925
Review-Url: https://codereview.webrtc.org/3006713002
Cr-Commit-Position: refs/heads/master@{#19600}
Committed: https://chromium.googlesource.com/external/webrtc/+/a35df4260f2e51f35e94fa74d1e840f73b597369
Patch Set 1 #Patch Set 2 : Fix #
Total comments: 2
Patch Set 3 : Change name from is_external to is_hardware_accelerated #
Messages
Total messages: 28 (20 generated)
The CQ bit was checked by magjed@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Patchset #1 (id:1) has been deleted
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_rel on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_rel/builds/29090)
Description was changed from ========== WebRtcVideoEngine: Encapsulate logic for unifying internal and external video codecs This CL encapsulates the logic for unifying the internal and external video encoders into a helper class. The purpose is to prepare for introducing a new video encoder factory interface that inherently represents all encoders (i.e. both internal and external). A helper interface EncoderFactoryAdapter is introduced that both the old WebRtcVideoEncoderFactory and the new factory interface can implement and serves as common point to leave the rest of the code unchanged. BUG=webrtc:7925 ========== to ========== WebRtcVideoEngine: Encapsulate logic for unifying internal and external video codecs This CL encapsulates the logic for unifying the internal and external video encoders into a helper class. The purpose is to prepare for introducing a new video encoder factory interface that inherently represents all encoders (i.e. both internal and external). A helper interface EncoderFactoryAdapter is introduced that both the old WebRtcVideoEncoderFactory and the new factory interface can implement and serves as common point to leave the rest of the code unchanged. BUG=webrtc:7925 ==========
magjed@webrtc.org changed reviewers: + andersc@webrtc.org
Anders - please take a look. I still have some bug with this CL I need to figure out, but this is easier to understand than the huge WIP CL.
The CQ bit was checked by magjed@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was checked by magjed@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
Patchset #2 (id:40001) has been deleted
Ok, now it's actually working. Please review.
lgtm https://codereview.webrtc.org/3006713002/diff/60001/webrtc/media/engine/webrt... File webrtc/media/engine/webrtcvideoengine.cc (right): https://codereview.webrtc.org/3006713002/diff/60001/webrtc/media/engine/webrt... webrtc/media/engine/webrtcvideoengine.cc:59: bool is_external; Not sure if this is a good point to start changing the terminology, but should this eventually be `is_hardware` instead?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.webrtc.org/3006713002/diff/60001/webrtc/media/engine/webrt... File webrtc/media/engine/webrtcvideoengine.cc (right): https://codereview.webrtc.org/3006713002/diff/60001/webrtc/media/engine/webrt... webrtc/media/engine/webrtcvideoengine.cc:59: bool is_external; On 2017/08/29 14:48:54, andersc wrote: > Not sure if this is a good point to start changing the terminology, but should > this eventually be `is_hardware` instead? Exactly, probably best to change it now.
The CQ bit was checked by magjed@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from andersc@webrtc.org Link to the patchset: https://codereview.webrtc.org/3006713002/#ps80001 (title: "Change name from is_external to is_hardware_accelerated")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_arm64_dbg on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_arm64_dbg/builds/...)
The CQ bit was checked by magjed@webrtc.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...
CQ is committing da patch.
Bot data: {"patchset_id": 80001, "attempt_start_ts": 1504091808794830,
"parent_rev": "386449971a8f471e87f3844ebee1b1bc74144998", "commit_rev":
"a35df4260f2e51f35e94fa74d1e840f73b597369"}
Message was sent while issue was closed.
Description was changed from ========== WebRtcVideoEngine: Encapsulate logic for unifying internal and external video codecs This CL encapsulates the logic for unifying the internal and external video encoders into a helper class. The purpose is to prepare for introducing a new video encoder factory interface that inherently represents all encoders (i.e. both internal and external). A helper interface EncoderFactoryAdapter is introduced that both the old WebRtcVideoEncoderFactory and the new factory interface can implement and serves as common point to leave the rest of the code unchanged. BUG=webrtc:7925 ========== to ========== WebRtcVideoEngine: Encapsulate logic for unifying internal and external video codecs This CL encapsulates the logic for unifying the internal and external video encoders into a helper class. The purpose is to prepare for introducing a new video encoder factory interface that inherently represents all encoders (i.e. both internal and external). A helper interface EncoderFactoryAdapter is introduced that both the old WebRtcVideoEncoderFactory and the new factory interface can implement and serves as common point to leave the rest of the code unchanged. BUG=webrtc:7925 Review-Url: https://codereview.webrtc.org/3006713002 Cr-Commit-Position: refs/heads/master@{#19600} Committed: https://chromium.googlesource.com/external/webrtc/+/a35df4260f2e51f35e94fa74d... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:80001) as https://chromium.googlesource.com/external/webrtc/+/a35df4260f2e51f35e94fa74d... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
