gpu-video-wallpaper icon indicating copy to clipboard operation
gpu-video-wallpaper copied to clipboard

Added pkill command for mpv and xwinwrap

Open NayamAmarshe opened this issue 2 years ago • 2 comments

Currently, the kill by pid doesn't seem to work. I don't know if my method of using pkill with the names of applications is better or not, but it does seem to work just fine. This makes sure that multiple instances of mpv and xwinwrap do not spawn, which was a problem earlier.

NayamAmarshe avatar Feb 16 '22 19:02 NayamAmarshe

Are you sure kill by pid does not work post-merge? Please refresh your sources try again. Also, if it still won't work for you, please provide your system details (distro, DE, etc.). For me (LM 20 x64 / Cinnamon), kill by pid works fine.

Generally speaking, pkill is too much of a sledgehammer method, which is why I removed it (or a similar method; can't remember) from the original video-wallpaper.sh and shifted the script towards kill by pid. If you use pkill, it will kill all instances of mpv, including those the user still wanted to keep running (e.g. for music or video playback).

One more minor niggle: You've duplicated code in your pull request; the start() function should not kill mpv and xwinwrap itself, but instead invoke the stop() function.

SwallowYourDreams avatar Feb 22 '22 12:02 SwallowYourDreams

Are you sure kill by pid does not work post-merge? Please refresh your sources try again. Also, if it still won't work for you, please provide your system details (distro, DE, etc.). For me (LM 20 x64 / Cinnamon), kill by pid works fine.

Generally speaking, pkill is too much of a sledgehammer method, which is why I removed it (or a similar method; can't remember) from the original video-wallpaper.sh and shifted the script towards kill by pid. If you use pkill, it will kill all instances of mpv, including those the user still wanted to keep running (e.g. for music or video playback).

One more minor niggle: You've duplicated code in your pull request; the start() function should not kill mpv and xwinwrap itself, but instead invoke the stop() function.

I'll try the new commit and let you know. The pid method was not working on my ZorinOS installation.

NayamAmarshe avatar Feb 22 '22 14:02 NayamAmarshe