Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add feature: select display to mirror #1238

Closed
wants to merge 4 commits into from
Closed

Add feature: select display to mirror #1238

wants to merge 4 commits into from

Conversation

peanutwolf
Copy link
Contributor

This is rebased version of #1177 PR

@rom1v I was in the middle of rebasing #1177 and accidentally deleted remote branch from my fork repository but pushed it back again. Unfortunately github doesn't allow to reopen my previous PR(button is disabled). Is it possible for you as a maintained to reopen #1177 and close this one to preserve the conversation history?

@rom1v
Copy link
Collaborator

rom1v commented Mar 28, 2020

Thank you 👍

I squashed the commits, and applied several changes locally. The result is on branch pr1238.

In particular, setDisplayId() is not called if displayId is 0 (the method might not exist).

Could you please review the changes, and test?

I don't understand what FLAG_SUPPORTS_PROTECTED_BUFFERS does? Is it important to check it (just to print a warning)?

@rom1v
Copy link
Collaborator

rom1v commented Mar 28, 2020

When the display does not exist, the server terminates before connecting to the client.

In that case, the client awaited on accept() indefinitely (and we had to Ctrl+C).

It's a problem I have known for a long time, but I never took the time to fix it. I just did it (but I tested only on Linux).

Could you please test (+review) the branch threadwait (it includes your PR) on Windows or Mac? Attempt to execute scrcpy --display 1234 (an invalid display id). It should terminate properly immediately.

Thank you :)

@rom1v
Copy link
Collaborator

rom1v commented Mar 29, 2020

Arf, on threadwait, it does not start on Windows 😞

(select() does not work for pipes on Windows)

@rom1v
Copy link
Collaborator

rom1v commented Mar 29, 2020

I fixed the problem a bit differently. Could you test threadwait.2 please?

@peanutwolf
Copy link
Contributor Author

I've test changes on threadwait.2 and it works fine for display_id and server termination features. But the thing is I have Linux machine too and can't test it on Mac or Windows, sorry.

I don't know how exactly MediaCodec works under the hood but what I have noticed is that if display doesn't have FLAG_SUPPORTS_PROTECTED_BUFFERS then android.media.MediaCodec#dequeueOutputBuffer is never called. Android doc says that absence of this flag means that display is unable to form secure buffers. So maybe it is worth terminating mirroring if display does not have this flag. I couldn't find clear connection, so that is why I used warning instead.

@rom1v
Copy link
Collaborator

rom1v commented Mar 30, 2020

Android doc says that absence of this flag means that display is unable to form secure buffers.

Oh, what if you pass false here:

return SurfaceControl.createDisplay("scrcpy", true);

?

@peanutwolf
Copy link
Contributor Author

I tested it but it has the same behavior even if i pass 'false' argument.

rom1v pushed a commit that referenced this pull request Apr 2, 2020
Add --display command line parameter to specify a display id.

PR #1238 <#1238>

Signed-off-by: Romain Vimont <[email protected]>
@rom1v
Copy link
Collaborator

rom1v commented Apr 2, 2020

Merged on dev branch: 4150eed

Thanks 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants