[WebRTC] Two DirectX capturers cannot work concurrently

[WebRTC] Two DirectX capturers cannot work concurrently

DirectX capturer won't be able to update the DesktopFrame for the second
ScreenCapturerWinDirectx instance, if AcquireNextFrame() returns timeout or
unchanged error. Current solution uses |last_frame| of the second
ScreenCapturerWinDirectx instance, which also does not contain the updated frame
between this and last AcquireNextFrame() calls. Considering following situation,
(C1: capturer 1, C2: capturer 2, update: screen updated, next AcquireNextFrame()
call will return a new frame, Fx: a frame x)
update -> C2.capture returns F1 -> update -> C1.capture returns F2 ->
C2.capture unchanged
So using F1 to update the last capture frame is not correct, we need to use F2.
Refer to design doc https://goo.gl/hU1ifG for a detail description.

The change also makes DxgiDuplicatorController work with 2+ DesktopFrame queue.
Now TwoDirectxCapturers test can pass.

BUG=314516
Committed: https://crrev.com/aa90b313b437df0043d0c1621f1175727f6145f6
Cr-Commit-Position: refs/heads/master@{#14077}

[Hzj_jie, 2016-08-31 23:08:04]

Patchset #1 (id:1) has been deleted

[Hzj_jie, 2016-08-31 23:12:22]

Patchset #1 (id:20001) has been deleted

[Hzj_jie, 2016-08-31 23:31:40]

Patchset #1 (id:40001) has been deleted

[Hzj_jie, 2016-08-31 23:42:18]

Patchset #1 (id:60001) has been deleted

[Hzj_jie, 2016-08-31 23:56:03]

Patchset #1 (id:80001) has been deleted

[Hzj_jie, 2016-09-01 03:07:49]

Description was changed from

==========
[WebRTC] Two DirectX capturers cannot work concurrently

DirectX capturer won't be able to update the DesktopFrame for the second
ScreenCapturerWinDirectx instance, if AcquireNextFrame() returns timeout or
unchanged error. Current solution uses |last_frame| of the second
ScreenCapturerWinDirectx instance, which also does not contain the updated frame
between this and last AcquireNextFrame() calls. Considering following situation,
(C1: capturer 1, C2: capturer 2, update: screen updated, next AcquireNextFrame()
call will return a new frame, Fx: a frame x)
update -> C2.capture returns F1 -> update -> C1.capture returns F2 ->
C2.capture unchanged
So using F1 to update the last capture frame is not correct, we need to use F2.
Refer to design doc https://goo.gl/hU1ifG for a detail description.

The change also makes DxgiDuplicatorController work with 2+ DesktopFrame queue.
Now TwoDirectxCapturers test can pass.

BUG=314516
==========

to

==========
[WebRTC] Two DirectX capturers cannot work concurrently

DirectX capturer won't be able to update the DesktopFrame for the second
ScreenCapturerWinDirectx instance, if AcquireNextFrame() returns timeout or
unchanged error. Current solution uses |last_frame| of the second
ScreenCapturerWinDirectx instance, which also does not contain the updated frame
between this and last AcquireNextFrame() calls. Considering following situation,
(C1: capturer 1, C2: capturer 2, update: screen updated, next AcquireNextFrame()
call will return a new frame, Fx: a frame x)
update -> C2.capture returns F1 -> update -> C1.capture returns F2 ->
C2.capture unchanged
So using F1 to update the last capture frame is not correct, we need to use F2.
Refer to design doc https://goo.gl/hU1ifG for a detail description.

The change also makes DxgiDuplicatorController work with 2+ DesktopFrame queue.
Now TwoDirectxCapturers test can pass.

BUG=314516
==========

[Hzj_jie, 2016-09-01 03:07:49]

zijiehe@chromium.org changed reviewers:
+ jamiewalch@chromium.org, sergeyu@chromium.org

[Hzj_jie, 2016-09-01 21:19:28]

TwoDirectxCapturers test will be enabled after CL/2268093002 has been submitted.

[Jamie, 2016-09-02 23:11:23]

I'm not an OWNER for WebRTC, so I'll let Sergey handle this one.

[Sergey Ulanov, 2016-09-02 23:57:35]

lgtm when my comments are addressed

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
File webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc (right):

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc:174:
context->updated_region.Clear();
Don't need this anymore

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc:175: for
(DesktopRegion::Iterator it(updated_region); !it.IsAtEnd();
This duplicates the code that you have above in lines 160-168. You always want
to copy from last_frame_. Remove return from line 168 and always run this code?

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
File webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h (right):

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h:69: const
DesktopVector& offset,
Don't need &. DesktopVector is small, just 8 bytes. The reference is also 8
bytes. So you are not achieving anything by passing a reference here. Also
remove const. Same for other functions below

[Hzj_jie, 2016-09-04 22:13:17]

Patchset #2 (id:120001) has been deleted

[Hzj_jie, 2016-09-05 21:24:22]

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
File webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc (right):

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc:174:
context->updated_region.Clear();
On 2016/09/02 23:57:35, Sergey Ulanov wrote:
> Don't need this anymore

Done.

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
webrtc/modules/desktop_capture/win/dxgi_output_duplicator.cc:175: for
(DesktopRegion::Iterator it(updated_region); !it.IsAtEnd();
On 2016/09/02 23:57:35, Sergey Ulanov wrote:
> This duplicates the code that you have above in lines 160-168. You always want
> to copy from last_frame_. Remove return from line 168 and always run this
code?

The code between 160 - 168 copies from texture_->AsDesktopFrame(). This is do a
little bit confusing.

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
File webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h (right):

https://codereview.chromium.org/2299663003/diff/100001/webrtc/modules/desktop...
webrtc/modules/desktop_capture/win/dxgi_output_duplicator.h:69: const
DesktopVector& offset,
On 2016/09/02 23:57:35, Sergey Ulanov wrote:
> Don't need &. DesktopVector is small, just 8 bytes. The reference is also 8
> bytes. So you are not achieving anything by passing a reference here. Also
> remove const. Same for other functions below

I used to use const to ensure this parameter won't be changed in the logic.
Done.

[Hzj_jie, 2016-09-05 21:24:38]

The CQ bit was checked by zijiehe@chromium.org

[Hzj_jie, 2016-09-05 21:24:38]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/2299663003/#ps140001
(title: "Resolve review comments")

[commit-bot: I haz the power, 2016-09-05 21:24:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-05 21:27:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-05 21:27:17]

Try jobs failed on following builders:
  presubmit on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/8062)

[Hzj_jie, 2016-09-05 22:02:01]

The CQ bit was checked by zijiehe@chromium.org

[Hzj_jie, 2016-09-05 22:02:01]

The patchset sent to the CQ was uploaded after l-g-t-m from sergeyu@chromium.org
Link to the patchset: https://codereview.chromium.org/2299663003/#ps160001
(title: "Sync latest changes")

[commit-bot: I haz the power, 2016-09-05 22:02:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-09-05 22:40:02]

Description was changed from

==========
[WebRTC] Two DirectX capturers cannot work concurrently

DirectX capturer won't be able to update the DesktopFrame for the second
ScreenCapturerWinDirectx instance, if AcquireNextFrame() returns timeout or
unchanged error. Current solution uses |last_frame| of the second
ScreenCapturerWinDirectx instance, which also does not contain the updated frame
between this and last AcquireNextFrame() calls. Considering following situation,
(C1: capturer 1, C2: capturer 2, update: screen updated, next AcquireNextFrame()
call will return a new frame, Fx: a frame x)
update -> C2.capture returns F1 -> update -> C1.capture returns F2 ->
C2.capture unchanged
So using F1 to update the last capture frame is not correct, we need to use F2.
Refer to design doc https://goo.gl/hU1ifG for a detail description.

The change also makes DxgiDuplicatorController work with 2+ DesktopFrame queue.
Now TwoDirectxCapturers test can pass.

BUG=314516
==========

to

==========
[WebRTC] Two DirectX capturers cannot work concurrently

DirectX capturer won't be able to update the DesktopFrame for the second
ScreenCapturerWinDirectx instance, if AcquireNextFrame() returns timeout or
unchanged error. Current solution uses |last_frame| of the second
ScreenCapturerWinDirectx instance, which also does not contain the updated frame
between this and last AcquireNextFrame() calls. Considering following situation,
(C1: capturer 1, C2: capturer 2, update: screen updated, next AcquireNextFrame()
call will return a new frame, Fx: a frame x)
update -> C2.capture returns F1 -> update -> C1.capture returns F2 ->
C2.capture unchanged
So using F1 to update the last capture frame is not correct, we need to use F2.
Refer to design doc https://goo.gl/hU1ifG for a detail description.

The change also makes DxgiDuplicatorController work with 2+ DesktopFrame queue.
Now TwoDirectxCapturers test can pass.

BUG=314516
==========

[commit-bot: I haz the power, 2016-09-05 22:40:03]

Committed patchset #3 (id:160001)

[commit-bot: I haz the power, 2016-09-05 22:40:11]

Description was changed from

==========
[WebRTC] Two DirectX capturers cannot work concurrently

DirectX capturer won't be able to update the DesktopFrame for the second
ScreenCapturerWinDirectx instance, if AcquireNextFrame() returns timeout or
unchanged error. Current solution uses |last_frame| of the second
ScreenCapturerWinDirectx instance, which also does not contain the updated frame
between this and last AcquireNextFrame() calls. Considering following situation,
(C1: capturer 1, C2: capturer 2, update: screen updated, next AcquireNextFrame()
call will return a new frame, Fx: a frame x)
update -> C2.capture returns F1 -> update -> C1.capture returns F2 ->
C2.capture unchanged
So using F1 to update the last capture frame is not correct, we need to use F2.
Refer to design doc https://goo.gl/hU1ifG for a detail description.

The change also makes DxgiDuplicatorController work with 2+ DesktopFrame queue.
Now TwoDirectxCapturers test can pass.

BUG=314516
==========

to

==========
[WebRTC] Two DirectX capturers cannot work concurrently

DirectX capturer won't be able to update the DesktopFrame for the second
ScreenCapturerWinDirectx instance, if AcquireNextFrame() returns timeout or
unchanged error. Current solution uses |last_frame| of the second
ScreenCapturerWinDirectx instance, which also does not contain the updated frame
between this and last AcquireNextFrame() calls. Considering following situation,
(C1: capturer 1, C2: capturer 2, update: screen updated, next AcquireNextFrame()
call will return a new frame, Fx: a frame x)
update -> C2.capture returns F1 -> update -> C1.capture returns F2 ->
C2.capture unchanged
So using F1 to update the last capture frame is not correct, we need to use F2.
Refer to design doc https://goo.gl/hU1ifG for a detail description.

The change also makes DxgiDuplicatorController work with 2+ DesktopFrame queue.
Now TwoDirectxCapturers test can pass.

BUG=314516

Committed: https://crrev.com/aa90b313b437df0043d0c1621f1175727f6145f6
Cr-Commit-Position: refs/heads/master@{#14077}
==========

[commit-bot: I haz the power, 2016-09-05 22:40:12]

Patchset 3 (id:??) landed as
https://crrev.com/aa90b313b437df0043d0c1621f1175727f6145f6
Cr-Commit-Position: refs/heads/master@{#14077}