Actions on start exit #3191
Actions on start exit #3191
Conversation
Find and replace all matches in a string
Unlike what would be expected, adb shell just concatenates the arguments as strings same as if they were just separated by spaces. Sending the commands in one argument or sending as multiple arguments doesn't preduce any different outcome. Sad but that's how adb works. try: adb shell 'am broadcast' vs adb shell am broadcast If they both work, means there's no reasonable way to deliver the arguments as-is. They have to be escaped.
brunoais
force-pushed
the
actions_on_start_exit
branch
from
9b29cec
to
4287759
Compare
| @@ -233,6 +234,23 @@ execute_server(struct sc_server *server, | |||
| if (params->encoder_name) { | |||
| ADD_PARAM("encoder_name=%s", params->encoder_name); | |||
| } | |||
| if (params->hook_script) { | |||
| char* requoted_hook; | |||
| int64_t replace_result = sc_str_find_replace(params->hook_script, "'", "'\"'\"'", &requoted_hook); | |||
There was a problem hiding this comment.
Unlike what would be expected, adb shell just concatenates the arguments as strings same
as if they were just separated by spaces. Sending the commands in one argument
or sending as multiple arguments doesn't preduce any different outcome.
Sad but that's how adb works.
try typing this in your local shell (you may need the -s argument for adb):
adb shell 'am broadcast'
vs
adb shell am broadcast
If they both work, means there's no reasonable way to deliver the arguments as-is, even when using execvp (which scrcpy is using).
Sadly, they have to be escaped.
Whether the escape be done here or before the calling stack to call adb shell is up for debate. However, no doubt it's needed and no doubt the behavior adb has is improper when intending to provide arguments as-is
There was a problem hiding this comment.
Unlike what would be expected, adb shell just concatenates the arguments as strings same
as if they were just separated by spaces
In fact, it depends on the adb version and the platform. It's a total mess. You can't rely on any specific behavior here. Adding quotes might break on some platforms and some adb versions.
Anyway, in practice, the command line should not exceed 256 chars (it crashes on some devices), so the script content could not be passed as a server argument.
There was a problem hiding this comment.
In that case, should I add sending the script to control_msg.c by queueing a message using sc_controller_push_msg() sometime in the beginning of the program execution while also implementing a new type at sc_control_msg_serialize() for that new feature (and then adding the counter-part code in the server)?
There was a problem hiding this comment.
I don't know what to do. I have mixed feelings about this feature.
should I add sending the script to
control_msg.cby queueing a message usingsc_controller_push_msg()
This would require to start the CleanUp process only after this message is received :/
There was a problem hiding this comment.
I don't know what to do. I have mixed feelings about this feature.
What about sending much of the first data through stdin? It can be encoded in some way and then just sent to stdin when adb shell is run.
With that, we can also deal with the issue of limited size of args, the argv inconsistency, etc... and just send all arguments through a stream in stdin as soon as the stdin stream exists. Maybe just encoding as base64 is enough and then ending with maybe the '\0'
This would require to start the CleanUp process only after this message is received :/
Only if a script exists but yes, if a script was set, it would need to wait.
There was a problem hiding this comment.
@rom1v Any answers you can give before I try again? It's been a few weeks
There was a problem hiding this comment.
@rom1v May I proceed to try to implement what I proposed? So far I can tell it appears to work.
There was a problem hiding this comment.
I have mixed feelings about this. IMO this feature is secondary, it adds complexity and requires to change how the parameters are passed, and apparently it does not behave the same way on all adb/devices. So TBH I'm not very keen it on for the moment :/
There was a problem hiding this comment.
Alright. In that case this and the alternative way that was already shot down are shot down.
I'll see if I can find an alternative way to process this.
Maybe it can be sent as a ControlMessage message like most features are done. Possibly in a similar way that the clipboard message is sent.
However, it would be convenient to increase the limit to 1«19 512KB. That way a script can be ~400KB which would allow a lot of flexibility.
I think that the downside of using the ControlMessage API is acceptable with that upside of stability between versions.
What do you think?
Allow running arbitrary scripts on start and exit of scrcpy.
Do note the exit script is not guaranteed to run depending on the phone you use and the way scrcpy exits.
See: #2265 (comment)
Replaces #2265