Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2743233002:
Ignore unmoved moved_rects in DxgiOutputDuplicator (Closed)
|
Created:
3 years, 9 months ago by Hzj_jie Modified:
3 years, 9 months ago Reviewers:
Sergey Ulanov CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Target Ref:
refs/heads/master Project:
webrtc Visibility:
Public. |
DescriptionIgnore unmoved moved_rects in DxgiOutputDuplicator
I cannot even imagine this change is useful. But it consistently reduces average
capture time by 0.375% (4.07 -> 4.055), and average encode time by 0.313%
(8.042 -> 8.016) without other impacts. Considering this is a one-line change,
it's worthy to be added.
BUG=679523
Review-Url: https://codereview.webrtc.org/2743233002
Cr-Commit-Position: refs/heads/master@{#17235}
Committed: https://chromium.googlesource.com/external/webrtc/+/bc935b4c495173c48bd74882b1350bb6f5fd1c36
Patch Set 1 #
Total comments: 2
Patch Set 2 : More comments and log all unmoved move_rects. #Messages
Total messages: 21 (16 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.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: presubmit on master.tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/14790)
Description was changed from ========== Ignore unmoved moved_rects BUG= ========== to ========== Ignore unmoved moved_rects in DxgiOutputDuplicator I cannot even imagine this change is useful. But it consistently reduces average capture time by 0.375% (4.07 -> 4.055), and average encode time by 0.313% (8.042 -> 8.016) without other impacts. Considering this is a two-line change, it's worth to be added. BUG=679523 ==========
zijiehe@chromium.org changed reviewers: + sergeyu@chromium.org
Description was changed from ========== Ignore unmoved moved_rects in DxgiOutputDuplicator I cannot even imagine this change is useful. But it consistently reduces average capture time by 0.375% (4.07 -> 4.055), and average encode time by 0.313% (8.042 -> 8.016) without other impacts. Considering this is a two-line change, it's worth to be added. BUG=679523 ========== to ========== Ignore unmoved moved_rects in DxgiOutputDuplicator I cannot even imagine this change is useful. But it consistently reduces average capture time by 0.375% (4.07 -> 4.055), and average encode time by 0.313% (8.042 -> 8.016) without other impacts. Considering this is a one-line change, it's worth to be added. BUG=679523 ==========
Description was changed from ========== Ignore unmoved moved_rects in DxgiOutputDuplicator I cannot even imagine this change is useful. But it consistently reduces average capture time by 0.375% (4.07 -> 4.055), and average encode time by 0.313% (8.042 -> 8.016) without other impacts. Considering this is a one-line change, it's worth to be added. BUG=679523 ========== to ========== Ignore unmoved moved_rects in DxgiOutputDuplicator I cannot even imagine this change is useful. But it consistently reduces average capture time by 0.375% (4.07 -> 4.055), and average encode time by 0.313% (8.042 -> 8.016) without other impacts. Considering this is a one-line change, it's worthy to be added. BUG=679523 ==========
lgtm https://codereview.webrtc.org/2743233002/diff/1/webrtc/modules/desktop_captur... File webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc (right): https://codereview.webrtc.org/2743233002/diff/1/webrtc/modules/desktop_captur... webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc:291: if (move_rects->SourcePoint.x != move_rects->DestinationRect.left || Do you know why the API returns unmoved rects? Do you know what they correspond to (maybe there is a specific application that triggers that behavior, e.g. by generating noop move events)? I think just logging these rects would provide more information. In any case, please add a comment to explain why we need this.
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.webrtc.org/...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.webrtc.org/2743233002/diff/1/webrtc/modules/desktop_captur... File webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc (right): https://codereview.webrtc.org/2743233002/diff/1/webrtc/modules/desktop_captur... webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc:291: if (move_rects->SourcePoint.x != move_rects->DestinationRect.left || On 2017/03/14 19:47:27, Sergey Ulanov wrote: > Do you know why the API returns unmoved rects? Do you know what they correspond > to (maybe there is a specific application that triggers that behavior, e.g. by > generating noop move events)? I think just logging these rects would provide > more information. > In any case, please add a comment to explain why we need this. No idea about this. I just randomly tried this idea, and found the impact. I doubt this information is also from the video adapter or its driver, which may be because of the noop move operations from applications or Windows (window manager) itself. I have logged the unmoved move_rect for further investigation, though it's may not worthy. (0.015 ms is not so attractive.) I also copy the description in this change into to source code to explain the reason and impact.
The CQ bit was checked by zijiehe@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org Link to the patchset: https://codereview.webrtc.org/2743233002/#ps20001 (title: "More comments and log all unmoved move_rects.")
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": 20001, "attempt_start_ts": 1489537534965950,
"parent_rev": "a613eb6bff33b62fef0589b2e4e587380ae23411", "commit_rev":
"bc935b4c495173c48bd74882b1350bb6f5fd1c36"}
Message was sent while issue was closed.
Description was changed from ========== Ignore unmoved moved_rects in DxgiOutputDuplicator I cannot even imagine this change is useful. But it consistently reduces average capture time by 0.375% (4.07 -> 4.055), and average encode time by 0.313% (8.042 -> 8.016) without other impacts. Considering this is a one-line change, it's worthy to be added. BUG=679523 ========== to ========== Ignore unmoved moved_rects in DxgiOutputDuplicator I cannot even imagine this change is useful. But it consistently reduces average capture time by 0.375% (4.07 -> 4.055), and average encode time by 0.313% (8.042 -> 8.016) without other impacts. Considering this is a one-line change, it's worthy to be added. BUG=679523 Review-Url: https://codereview.webrtc.org/2743233002 Cr-Commit-Position: refs/heads/master@{#17235} Committed: https://chromium.googlesource.com/external/webrtc/+/bc935b4c495173c48bd74882b... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/external/webrtc/+/bc935b4c495173c48bd74882b... |
