kokoko3k / ssh-rdp

Real display ssh based remote desktop
GNU Lesser General Public License v3.0
141 stars 30 forks source link

[Feature request?] hooks to execute a script on the ssh server #23

Closed xenogenesi closed 4 months ago

xenogenesi commented 4 months ago

Thank you again for sharing this project, it's very helpful to me.

I use a touchscreen tablet to control a remote PC. For some applications, I need to rotate the screen and the coordinates of the remote PC before video encoding starts, after the xinput devices have been created, and reset the rotation upon exit.

Perhaps it needs a better naming for the option and variable (I'm a bit confused about what's remote being the ssh-rdp executed on the tablet).

I've created a patch. If you're interested, I can create a pull request.

diff --git a/ssh-rdp.sh b/ssh-rdp.sh
index 2acfa85..3206c2b 100755
--- a/ssh-rdp.sh
+++ b/ssh-rdp.sh
@@ -477,6 +477,9 @@ do
         --rexec-exit)
             REXEC_EXIT="$2"
             shift ; shift ;;
+       --exec-remote)
+           EXEC_REMOTE="$2"
+           shift ; shift ;;
         *)
             shift ;;
     esac
@@ -781,6 +784,11 @@ echo
         PID4=$!
     fi

+    if ! [ "$EXEC_REMOTE" = "" ] ; then
+       print_pending "Executing $EXEC_REMOTE start $RDISPLAY"
+       $SSH_EXEC sh "$EXEC_REMOTE start $RDISPLAY"
+       print_ok "$EXEC_REMOTE start exited."
+    fi

 #Grab Video
     print_pending "Start video streaming..."
@@ -814,3 +822,9 @@ echo
                 -vf hwmap=derive_device=vaapi,crop="$RES:$OFFSET",scale_vaapi="$NEWRES":format=nv12 -c:v h264_vaapi -bf 0  -b:v "$VIDEO_BITRATE_MAX"k  -maxrate "$VIDEO_BITRATE_MAX"k -f_strict experimental -syncpoints none  -f nut -\
                 " | $VIDEOPLAYER
     fi
+
+    if ! [ "$EXEC_REMOTE" = "" ] ; then
+       print_pending "Executing $EXEC_REMOTE stop $RDISPLAY"
+       $SSH_EXEC sh "$EXEC_REMOTE stop $RDISPLAY"
+       print_ok "$EXEC_REMOTE stop exited."
+    fi

It can be used with a similar script like this:

#!/bin/sh

# echo "$0: arguments: $@"

export DISPLAY=$2

if [ "$1" = "start" ]; then
    xrandr -o left
    while ! xinput|grep Goodix >/dev/null ; do
        sleep 1
    done
    xinput set-prop "pointer:Goodix Capacitive TouchScreen" --type=float "Coordinate Transformation Matrix" 0 -1 1 1 0 0 0 0 1
elif [ "$1" = "stop" ]; then
    xrandr -o normal
fi
kokoko3k commented 4 months ago

Hi there, I'm a bit confused; what's the difference with the existing --rexec-before/--rexec-after options?

xenogenesi commented 4 months ago

I have a tablet that I use as a client running ssh-rdp, and a PC that serves as the ssh server. When I tried the --rexec-* options, it seemed like they executed the script on the tablet. I ran hostname in the script and it gave me the tablet's hostname. However, I needed to run it on the ssh server.

Now that you ask this question, though, I have doubts. When using the script as input with ssh, does it resolve the variables before sending the script to the server? Because I invoked hostname as a subprocess $( ) (from the name, I expected the --rexec-* commands to be executed on the server, which caused some confusion). I think I run it also as a normal command but can't recall if it was mine version already.

Assuming that was the issue, to correct the touchscreen mapping with xinput, I need the forwarded xinput devices to be created on the server, and I wait for them with a loop and grep. Wouldn't REXEC_BEFORE be too early in the script for that kind of usage and wait forever?

kokoko3k commented 4 months ago

it does:

if ! [ "$REXEC_BEFORE" = "" ] ; then
    print_pending "Executing $REXEC_BEFORE"
    $SSH_EXEC "bash -s" < "$REXEC_BEFORE"
    print_ok "$REXEC_BEFORE exited."
fi

So much like the one you use, and probably it resolved the hostname var locally.

I understood the issue now, and yes, you're right, there's no way do that actually.

A pr would be good, but please follow the in place syntax!

xenogenesi commented 4 months ago

Hello!

Please check if it's okay, if there's anything you want to change just let me know, if you see something wrong with the descriptions in the README and the help, or if are too long. I'm not very convinced that exec-remote, start and stop are the right names/terms, maybe init and term would be better, for exec-remote I've not Idea, I just used it for speed while developing it, if you have any just tell and I'll change it.

https://github.com/kokoko3k/ssh-rdp/compare/master...xenogenesi:ssh-rdp:feat/exec-remote

PS: I'm not experienced with PRs, but I'll give it a try.

kokoko3k commented 4 months ago

Hi there, I feel this is a bit over-enginered; we already have rexec-stop which does almost the same as --exec-remote stop.

What about just adding a new --rexec-late which whould just start a script after input has been setup and re-use rexec-stop afterwards? Code should be just a copy/paste of the existing, just in the new position and no arguments should be allowed if not for the script itself.

xenogenesi commented 4 months ago

I had the same thought at some point, I can try, although I fear that since the script is executed first inside finish(), it will annoy the video encoding with the screen rotation. I'll look into it as soon as I find a moment. I'll let you know

xenogenesi commented 4 months ago

https://github.com/kokoko3k/ssh-rdp/compare/master...xenogenesi:ssh-rdp:feat/rexec-late

It works for what I need to do. The fact that the screen is rotated before the video encoding is finished doesn't seem to be a problem. I haven't noticed any significant differences compared to the other implementation. If this is okay, I'll proceed with the PR, otherwise, let me know what needs to be changed

kokoko3k commented 4 months ago

Clean, looks ok to me.

xenogenesi commented 4 months ago

Thanks! Bye