Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2092543003:
[Chromoting] DirectX based capturer should always return a temporary error (Closed)
Description[Chromoting] DirectX based capturer should always return a temporary error
When Windows is switching display mode, DirectX based capturer may not be able
to create a new IDXGIOutputDuplication instance, which is expected. So it should
return a temporary error instead of a permanent error.
BUG=
Committed: https://crrev.com/721ede1096d9c3afc8d846311692a1ac1888c712
Cr-Commit-Position: refs/heads/master@{#13279}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Resolve review comments #Messages
Total messages: 19 (8 generated)
Description was changed from ========== [Chromoting] DirectX based capturer should always return a temporary error When Windows is switching display mode, DirectX based capturer may not be able to create a new IDXGIOutputDuplication instance, which is expected. So it should return a temporary error instead of a permanent error. BUG= ========== to ========== [Chromoting] DirectX based capturer should always return a temporary error When Windows is switching display mode, DirectX based capturer may not be able to create a new IDXGIOutputDuplication instance, which is expected. So it should return a temporary error instead of a permanent error. BUG= ==========
zijiehe@chromium.org changed reviewers: + sergeyu@chromium.org
zijiehe@chromium.org changed reviewers: + jamiewalch@chromium.org
Are there other scenarios in which this can fail that would indicate a permanent error? Could you check GetLastError, for example? https://codereview.chromium.org/2092543003/diff/1/webrtc/modules/desktop_capt... File webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc (right): https://codereview.chromium.org/2092543003/diff/1/webrtc/modules/desktop_capt... webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc:623: // Windows is switching display mode. Retrying later usually resolve the s/resolve/resolves/
On 2016/06/23 17:16:32, Jamie wrote: > Are there other scenarios in which this can fail that would indicate a permanent > error? Could you check GetLastError, for example? > > https://codereview.chromium.org/2092543003/diff/1/webrtc/modules/desktop_capt... > File webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc (right): > > https://codereview.chromium.org/2092543003/diff/1/webrtc/modules/desktop_capt... > webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc:623: // > Windows is switching display mode. Retrying later usually resolve the > s/resolve/resolves/ In ScreenCapturer::Create (screen_capturer_win.cc), the ScreenCapturerDirectx::Initialize() will check all the software and hardware requirement. i.e. Anything wrong will cause the Initialize fail. But if initialization succeeded, I did not find any possibility it would return a permanent error. And Direct3D APIs are not consistent with other Windows APIs, GetLastError seems won't work with these APIs. I have gone through MSDN, none of the APIs I am using here may record an error which can be retrieved by GetLastError.
https://codereview.chromium.org/2092543003/diff/1/webrtc/modules/desktop_capt... File webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc (right): https://codereview.chromium.org/2092543003/diff/1/webrtc/modules/desktop_capt... webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc:623: // Windows is switching display mode. Retrying later usually resolve the On 2016/06/23 17:16:32, Jamie wrote: > s/resolve/resolves/ Done.
On 2016/06/23 18:37:51, Hzj_jie wrote: > On 2016/06/23 17:16:32, Jamie wrote: > > Are there other scenarios in which this can fail that would indicate a > permanent > > error? Could you check GetLastError, for example? > > > > > https://codereview.chromium.org/2092543003/diff/1/webrtc/modules/desktop_capt... > > File webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc > (right): > > > > > https://codereview.chromium.org/2092543003/diff/1/webrtc/modules/desktop_capt... > > webrtc/modules/desktop_capture/win/screen_capturer_win_directx.cc:623: // > > Windows is switching display mode. Retrying later usually resolve the > > s/resolve/resolves/ > > In ScreenCapturer::Create (screen_capturer_win.cc), the > ScreenCapturerDirectx::Initialize() will check all the software and hardware > requirement. i.e. Anything wrong will cause the Initialize fail. But if > initialization succeeded, I did not find any possibility it would return a > permanent error. And Direct3D APIs are not consistent with other Windows APIs, > GetLastError seems won't work with these APIs. I have gone through MSDN, none of > the APIs I am using here may record an error which can be retrieved by > GetLastError. Okay; thanks for being thorough. LGTM.
The CQ bit was checked by zijiehe@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2092543003/20001
The CQ bit was unchecked by commit-bot@chromium.org
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full committers or once the patch has received an L-G-T-M from a full committer. Even if an L-G-T-M may have been provided, it was from a non-committer, _not_ a full super star committer. Committers are members of the group "project-webrtc-committers". Note that this has nothing to do with OWNERS files.
lgtm
The CQ bit was checked by zijiehe@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2092543003/20001
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] DirectX based capturer should always return a temporary error When Windows is switching display mode, DirectX based capturer may not be able to create a new IDXGIOutputDuplication instance, which is expected. So it should return a temporary error instead of a permanent error. BUG= ========== to ========== [Chromoting] DirectX based capturer should always return a temporary error When Windows is switching display mode, DirectX based capturer may not be able to create a new IDXGIOutputDuplication instance, which is expected. So it should return a temporary error instead of a permanent error. BUG= ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [Chromoting] DirectX based capturer should always return a temporary error When Windows is switching display mode, DirectX based capturer may not be able to create a new IDXGIOutputDuplication instance, which is expected. So it should return a temporary error instead of a permanent error. BUG= ========== to ========== [Chromoting] DirectX based capturer should always return a temporary error When Windows is switching display mode, DirectX based capturer may not be able to create a new IDXGIOutputDuplication instance, which is expected. So it should return a temporary error instead of a permanent error. BUG= Committed: https://crrev.com/721ede1096d9c3afc8d846311692a1ac1888c712 Cr-Commit-Position: refs/heads/master@{#13279} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/721ede1096d9c3afc8d846311692a1ac1888c712 Cr-Commit-Position: refs/heads/master@{#13279} |
