Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2348943002:
[Chromoting] Use detect_updated_region option in remoting (Closed)
Description[Chromoting] Use detect_updated_region option in remoting
ScreenCapturer / DesktopCapturer do not always need to detect more accurate
updated region, since some consumers do not really care about the updated
region. And according to the analysis, a Differ call may use some 10 ms for a
2560 x 1440 DesktopFrame. So Differ logic will eventually be moved out of
ScreenCapturer / DesktopCapturer implementations.
But a more accurate updated region is useful for Chromoting, since we do not
need to send an unchanged frame to the client. So this change actively sets
detect_updated_region in DesktopCapturerOptions to ensure
ScreenCapturerDifferWrapper is used.
This change will be submitted together with WebRTC change
https://codereview.chromium.org/2348803003/ to avoid the differ to be involved
twice.
BUG=633802
Committed: https://crrev.com/3ba38ac9d3ae2b0d1d4d5230cd1280d417694390
Cr-Commit-Position: refs/heads/master@{#420863}
Patch Set 1 #
Messages
Total messages: 18 (10 generated)
The CQ bit was checked by zijiehe@chromium.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.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
zijiehe@chromium.org changed reviewers: + jamiewalch@chromium.org, sergeyu@chromium.org
LGTM, but please add something (to the code, if you think it's appropriate; to the CL description otherwise) explaining why this is important.
Description was changed from ========== [Chromoting] Use detect_updated_region option in remoting BUG=314516 ========== to ========== [Chromoting] Use detect_updated_region option in remoting ScreenCapturer / DesktopCapturer do not always need to detect more accurate updated region, since some consumers do not really care about the updated region. And according to the analysis, a Differ call may use some 10 ms for a 2560 x 1440 DesktopFrame. So Differ logic will eventually be moved out of ScreenCapturer / DesktopCapturer implementations. But a more accurate updated region is useful for Chromoting, since we do not need to send an unchanged frame to the client. So this change actively sets detect_updated_region in DesktopCapturerOptions to ensure ScreenCapturerDifferWrapper is used. This change will be submitted together with WebRTC change https://codereview.chromium.org/2348803003/ to avoid the differ to be involved twice. BUG=314516 ==========
On 2016/09/19 19:20:01, Jamie wrote: > LGTM, but please add something (to the code, if you think it's appropriate; to > the CL description otherwise) explaining why this is important. Sure, updated. I am waiting for the review of change https://codereview.chromium.org/2319383002/, to ensure this one and https://codereview.chromium.org/2348803003/ can be submitted together to avoid differ to be involved twice.
lgtm
Description was changed from ========== [Chromoting] Use detect_updated_region option in remoting ScreenCapturer / DesktopCapturer do not always need to detect more accurate updated region, since some consumers do not really care about the updated region. And according to the analysis, a Differ call may use some 10 ms for a 2560 x 1440 DesktopFrame. So Differ logic will eventually be moved out of ScreenCapturer / DesktopCapturer implementations. But a more accurate updated region is useful for Chromoting, since we do not need to send an unchanged frame to the client. So this change actively sets detect_updated_region in DesktopCapturerOptions to ensure ScreenCapturerDifferWrapper is used. This change will be submitted together with WebRTC change https://codereview.chromium.org/2348803003/ to avoid the differ to be involved twice. BUG=314516 ========== to ========== [Chromoting] Use detect_updated_region option in remoting ScreenCapturer / DesktopCapturer do not always need to detect more accurate updated region, since some consumers do not really care about the updated region. And according to the analysis, a Differ call may use some 10 ms for a 2560 x 1440 DesktopFrame. So Differ logic will eventually be moved out of ScreenCapturer / DesktopCapturer implementations. But a more accurate updated region is useful for Chromoting, since we do not need to send an unchanged frame to the client. So this change actively sets detect_updated_region in DesktopCapturerOptions to ensure ScreenCapturerDifferWrapper is used. This change will be submitted together with WebRTC change https://codereview.chromium.org/2348803003/ to avoid the differ to be involved twice. BUG=633802 ==========
The CQ bit was checked by zijiehe@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] Use detect_updated_region option in remoting ScreenCapturer / DesktopCapturer do not always need to detect more accurate updated region, since some consumers do not really care about the updated region. And according to the analysis, a Differ call may use some 10 ms for a 2560 x 1440 DesktopFrame. So Differ logic will eventually be moved out of ScreenCapturer / DesktopCapturer implementations. But a more accurate updated region is useful for Chromoting, since we do not need to send an unchanged frame to the client. So this change actively sets detect_updated_region in DesktopCapturerOptions to ensure ScreenCapturerDifferWrapper is used. This change will be submitted together with WebRTC change https://codereview.chromium.org/2348803003/ to avoid the differ to be involved twice. BUG=633802 ========== to ========== [Chromoting] Use detect_updated_region option in remoting ScreenCapturer / DesktopCapturer do not always need to detect more accurate updated region, since some consumers do not really care about the updated region. And according to the analysis, a Differ call may use some 10 ms for a 2560 x 1440 DesktopFrame. So Differ logic will eventually be moved out of ScreenCapturer / DesktopCapturer implementations. But a more accurate updated region is useful for Chromoting, since we do not need to send an unchanged frame to the client. So this change actively sets detect_updated_region in DesktopCapturerOptions to ensure ScreenCapturerDifferWrapper is used. This change will be submitted together with WebRTC change https://codereview.chromium.org/2348803003/ to avoid the differ to be involved twice. BUG=633802 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] Use detect_updated_region option in remoting ScreenCapturer / DesktopCapturer do not always need to detect more accurate updated region, since some consumers do not really care about the updated region. And according to the analysis, a Differ call may use some 10 ms for a 2560 x 1440 DesktopFrame. So Differ logic will eventually be moved out of ScreenCapturer / DesktopCapturer implementations. But a more accurate updated region is useful for Chromoting, since we do not need to send an unchanged frame to the client. So this change actively sets detect_updated_region in DesktopCapturerOptions to ensure ScreenCapturerDifferWrapper is used. This change will be submitted together with WebRTC change https://codereview.chromium.org/2348803003/ to avoid the differ to be involved twice. BUG=633802 ========== to ========== [Chromoting] Use detect_updated_region option in remoting ScreenCapturer / DesktopCapturer do not always need to detect more accurate updated region, since some consumers do not really care about the updated region. And according to the analysis, a Differ call may use some 10 ms for a 2560 x 1440 DesktopFrame. So Differ logic will eventually be moved out of ScreenCapturer / DesktopCapturer implementations. But a more accurate updated region is useful for Chromoting, since we do not need to send an unchanged frame to the client. So this change actively sets detect_updated_region in DesktopCapturerOptions to ensure ScreenCapturerDifferWrapper is used. This change will be submitted together with WebRTC change https://codereview.chromium.org/2348803003/ to avoid the differ to be involved twice. BUG=633802 Committed: https://crrev.com/3ba38ac9d3ae2b0d1d4d5230cd1280d417694390 Cr-Commit-Position: refs/heads/master@{#420863} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/3ba38ac9d3ae2b0d1d4d5230cd1280d417694390 Cr-Commit-Position: refs/heads/master@{#420863} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
