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

Added a new option : -n/--no-window #418

Closed

Conversation

TheCapsLock
Copy link
Contributor

@TheCapsLock TheCapsLock commented Feb 1, 2019

This option allows scrcpy to be run withou showing preview window : we can now use
scrcpy only for screen recording as an example

This option allows scrcpy to be run headless : we can now use
scrcpy only for screen recording as an example
Copy link
Collaborator

@rom1v rom1v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution.

I like the idea to add such a flag. Maybe it could skip a lot more code (do not decode frame, do not initialize SDL…).

I'll test it probably after the FOSDEM.

screen_show_window(&screen);
if (!no_window) {
screen_show_window(&screen);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can skip code far earlier, you may not even need to initialize SDL nor decode frames at all if you have no window.

app/src/main.c Outdated
@@ -209,6 +215,7 @@ static SDL_bool parse_args(struct args *args, int argc, char *argv[]) {
{"bit-rate", required_argument, NULL, 'b'},
{"crop", required_argument, NULL, 'c'},
{"fullscreen", no_argument, NULL, 'f'},
{"no-window", no_argument, NULL, 'n'},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alignment

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done; sorry for that

app/src/main.c Outdated
@@ -219,7 +226,7 @@ static SDL_bool parse_args(struct args *args, int argc, char *argv[]) {
{NULL, 0, NULL, 0 },
};
int c;
while ((c = getopt_long(argc, argv, "b:c:fhm:p:r:s:tv", long_options, NULL)) != -1) {
while ((c = getopt_long(argc, argv, "b:c:fnhm:p:r:s:tv", long_options, NULL)) != -1) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(please keep the alphabetical order, ditto above and below)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@npes87184
Copy link
Contributor

Hi,

I have two simple recommendations for this patch:

  • Please provide either a log in console or a toast in android side, so that we can know that the scrcpy finishes its initialization.
  • Use -n alone doesn’t make sense now. IMO, we can return error if -n and -r don’t exist simultaneously.

Thanks,
Yu-Chen

app/src/main.c Outdated
@@ -232,6 +239,9 @@ static SDL_bool parse_args(struct args *args, int argc, char *argv[]) {
case 'f':
args->fullscreen = SDL_TRUE;
break;
case 'n':
args->no_window = SDL_TRUE;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing default value or an initialization. See main.c

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@TheCapsLock
Copy link
Contributor Author

* Use -n alone doesn’t make sense now. IMO, we can return error if -n and -r don’t exist simultaneously.

Added an error message if nor -n is provided without -r

@TheCapsLock
Copy link
Contributor Author

Thank you for your contribution.

I like the idea to add such a flag. Maybe it could skip a lot more code (do not decode frame, do not initialize SDL…

I'll test it probably after the FOSDEM.

Thanks a lot for that ; I tried some combinations but did not get something better yet ; if you have clear idea in mind may I let you look at this specific topic ?

@TheCapsLock
Copy link
Contributor Author

* Please provide either a log in console or a toast in android side, so that we can know that the scrcpy finishes its initialization.

I've added a log message, thank you for this clever recommendation

@rom1v
Copy link
Collaborator

rom1v commented Feb 17, 2019

if you have clear idea in mind may I let you look at this specific topic ?

Yes, it requires some refactor beforehand: the decoder must be separated from the "video stream receiver", so that we can skip the decoder completely (we don't need to decode the video at all if there is no window). I will try to work on it so that it is included in the next version.

@TheCapsLock
Copy link
Contributor Author

if you have clear idea in mind may I let you look at this specific topic ?

Yes, it requires some refactor beforehand: the decoder must be separated from the "video stream receiver", so that we can skip the decoder completely (we don't need to decode the video at all if there is no window). I will try to work on it so that it is included in the next version.

Great to here this kind of news :) thanks a lot!

rom1v added a commit that referenced this pull request Mar 2, 2019
Disable decoder, screen (display), file_handler and controller when
--no-window is requested.

<#418>
@rom1v
Copy link
Collaborator

rom1v commented Mar 2, 2019

I refactored to split the decoder into stream+decoder: e6e011b.
Then I rebased your commit (but removed the part which disabled the window after decoding): 421a114.
Then I disabled everything useless when --no-window is passed: 89812e4.

It's on dev branch for now.

I'm closing as merged (even if github does see it as merged).

@rom1v rom1v closed this Mar 2, 2019
rom1v added a commit that referenced this pull request Mar 2, 2019
Disable decoder, screen (display), file_handler and controller when
--no-window is requested.

<#418>
@rom1v
Copy link
Collaborator

rom1v commented Mar 2, 2019

I have renamed the option -N/--no-display.

The description of scrcpy is "Display and control your Android device".

We want an option to disable display, another one to disable control, so I also implemented -n/--no-control.

@TheCapsLock
Copy link
Contributor Author

Tested on multiple devices, seems to work great, thanks ❤️

@Gapplay
Copy link

Gapplay commented Mar 22, 2019

@rom1v is possible to show fps when No-display is enabled?

@rom1v
Copy link
Collaborator

rom1v commented Mar 22, 2019

Currently, no.

A command line option (--print-fps) could be added to enable it on start.

@rom1v
Copy link
Collaborator

rom1v commented Mar 22, 2019

@Gapplay Please open a new issue for it, to not forget it.

@Gapplay Gapplay mentioned this pull request Mar 22, 2019
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.

4 participants