Vergenter commented 1 year ago

Pull Request Template

Type of change

[ ] Bug fix
[x] Feature
[ ] Documentation update

Description

This update provides a simpler method for Windows users to use the application when utilizing WSL2. A current issue exists with syncplay. It does not work correctly. Simple way to reproduce it is to run it from WSL2 using:

"/mnt/c/Program Files (x86)/Syncplay/syncplay.exe"

It silently fails if a URL is specified(even during runtime), causing the window to close unexpectedly. A workaround for this is: (not currently applied)

cmd.exe /C "C:\Program Files (x86)\Syncplay\syncplay.exe" "$URL"

Moreover, using uname -s to switch cases for syncplay is insufficient and requires modification.

-s | --syncplay)
            case "$(uname -s)" in
                Darwin*) player_function="/Applications/Syncplay.app/Contents/MacOS/syncplay" ;;
                MINGW* | *Msys)
                    export PATH="$PATH":"/c/Program Files (x86)/Syncplay/"
                    player_function="syncplay.exe"
                    ;;
                *) case "$(uname -r)" in
                    *WSL2*)
                        export PATH="$PATH:/mnt/c/Program Files (x86)/Syncplay/"
                        player_function="syncplay.exe"
                        ;;
                    *)
                        player_function="syncplay"
                        ;;
                esac

                ;; 
            esac
            ;;

The checks I've confirmed manually are already marked. Other checks, not yet verified, should function correctly given that only the display program is being modified. I will complete the rest after resolving the syncplay issue.

Checklist

[x] any anime playing
[x] bumped version
[x] next, prev and replay work
[x] -c history and continue work
[x] -d downloads work
[x] -s syncplay works
[x] -q quality works
[x] -v vlc works
[x] -e select episode works
[x] -S select index works
[x] -r range selection works
[x] --dub and regular (sub) mode both work
[x] all providers return links (not necessarily on a single anime, use debug mode to confirm)
[x] -h help info is up to date
[x] Readme is up to date
[x] Man page is up to date

Additional Testcases

The safe bet: One Piece
Episode 0: Saenai Heroine no Sodatekata ♭
Unicode: Saenai Heroine no Sodatekata ♭
Non-whole episodes: Tensei shitara slime datta ken (ep. 24.5, ep. 24.9)

port19x commented 1 year ago

I'm skeptical if we should explicitly support this in a documented fashion. Your changes to the ani-cli matching are welcome tho.

As you see some automated checks failed, clean those up and I'll do a more thorough review

justchokingaround commented 1 year ago

no need to add that to the README

Vergenter commented 1 year ago

Long story short:

`syncplay` treats links as files

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "syncplayClient.py", line 17, in <module>
  File "syncplay\ep_client.pyc", line 8, in main
  File "syncplay\clientManager.pyc", line 9, in run
  File "syncplay\ui\ConfigurationGetter.pyc", line 557, in getConfiguration
  File "syncplay\ui\ConfigurationGetter.pyc", line 471, in _loadRelativeConfiguration
  File "syncplay\ui\ConfigurationGetter.pyc", line 462, in __getRelativeConfigLocations
  File "ntpath.pyc", line 651, in realpath
  File "ntpath.pyc", line 601, in _getfinalpathname_nonstrict
FileNotFoundError: [WinError 161] The specified path is invalid: '\\\\wsl$\\Ubuntu\\home\\vergenter\\ani-cli\\https:\\www054.vipanicdn.net\\streamhls\\f134bc36b9bb59de2ef28a48a7cef6bf\\ep.5.1683732036.1080.m3u8'

The only workaround I found for this is to call it using cmd.exe (PowerShell also has a problem with the url-path). cmd.exe /C "syncplay.exe"

cmd call and argument(title) with spaces

I cannot use cmd in a way described below because then I cannot handle spaces in the title. WSL loses the ability to use the absolute path when escaping spaces in the title in any way.

vergenter@DESKTOP-JDQ0A50:/mnt/c$ cmd.exe /C "C:\Program Files (x86)\Syncplay\syncplay.exe" "123" --  --force-media-title="123 123"
'C:\Program' is not recognized as an internal or external command,
operable program or batch file.

Without a space in the title it works fine...

Solutions

Use the path for syncplay, but it needs to be the Windows path rather than the WSL path, as cmd won't use the WSL paths. This is rejected because Windows syncplay does not add itself to the path.
cd to the path. This is ugly, but it avoids all problems with the title. Additionally, checking dependencies for syncplay this way works correctly.
It should be now ready to review.

Should I modify this part? https://github.com/Vergenter/ani-cli/blob/ad2b2c00072612d4c4dbd203dc3a6536b37e6cbc/ani-cli#L346-L364

Vergenter commented 1 year ago

I'd rather not have such large code addition for an unsupported plattform. We support windows natively. The expanded matching for a wsl2 uname is welcome, but that's about the extend I'm willing to make concessions for wsl users.

Understood. I worked on adding syncplay support for WSL2 because it's on the required list.

However, note that currently syncplay is supported in weird/hacky way:

For some reason, it switches using a different uname.
Syncplay doesn't officially support URLs; it uses file paths as arguments.

justchokingaround commented 1 year ago

syncplay works with URLs, it just doesn't support m3u8 links

Vergenter commented 1 year ago

syncplay works with URLs, it just doesn't support m3u8 links


vergenter@DESKTOP-JDQ0A50:~$ syncplay --help
usage: syncplay [-h] [--no-gui] [-a hostname] [-n username] [-d] [-g] [--no-store] [-r [room]] [-p [password]]
[--player-path path] [--language language] [--clear-gui-data] [-v]
[--load-playlist-from-file loadPlaylistFromFile]
[file] [options ...]

Solution to synchronize playback of multiple media player instances over the network.

positional arguments: file file to play options player options, if you need to pass options starting with - prepend them with single '--' argument


First positional argument is a file and this is how it is used in ani-cli.
https://github.com/pystardust/ani-cli/blob/d4f9c22d5dad99a7c0ff169c89521d2976dc3cff/ani-cli#L258-L258
Using url in place of file is undocumented behaviour and it fails because of this in WSL.

BTW. Code is ready I applied change proposals.

justchokingaround commented 1 year ago

Using url in place of file is undocumented behaviour and it fails because of this in WSL.

wym? the "file" that is passed in ani-cli is literally a url. ik it because i implemented it. i'm telling you that links that end with .m3u8 do not work with syncplay, no matter which platform.

have you tested in wsl syncplay with .mp4 links?

Vergenter commented 1 year ago

wym? the "file" that is passed in ani-cli is literally a url. ik it because i implemented it. i'm telling you that links that end with .m3u8 do not work with syncplay, no matter which platform.

have you tested in wsl syncplay with .mp4 links?

As I previously mentioned WSL2 fails, because of it: That is from syncplay logs(URL is parsed like path):

FileNotFoundError: [WinError 161] The specified path is invalid: '\\\\wsl$\\Ubuntu\\home\\vergenter\\ani-cli\\https:\\www054.vipanicdn.net\\streamhls\\f134bc36b9bb59de2ef28a48a7cef6bf\\ep.5.1683732036.1080.m3u8'

justchokingaround commented 1 year ago

does the same thing happen with mp4 files?

Vergenter commented 1 year ago

does the same thing happen with mp4 files?

If by mp4 you mean for example this: obraz Then yes.

FileNotFoundError: [WinError 161] The specified path is invalid: '\\\\wsl$\\Ubuntu\\home\\vergenter\\ani-cli\\https:\\myanime.sharepoint.com\\sites\\chartlousty\\_layouts\\15'

This is filled correctly: obraz wsl prefix is not visible there: obraz If I start syncplay from windows and manually paste url it works ok. If I start syncplay from wsl and manually paste url it fails.

justchokingaround commented 1 year ago

does syncplay even work with any links that are not local files, in wsl?

justchokingaround commented 1 year ago

even youtube links

Vergenter commented 1 year ago

even youtube links

All URL fails using syncplay from wsl.(m3u8 from windows syncplay work ok, I just tested)

FileNotFoundError: [WinError 161] The specified path is invalid: '\\\\wsl$\\Ubuntu\\home\\vergenter\\ani-cli\\https:\\www.youtube.com'

justchokingaround commented 1 year ago

oh ok, then we ignore syncplay support for wsl

Vergenter commented 1 year ago

@justchokingaround How does in other situation url work at all? This is how it is handled in syncplay: https://github.com/Syncplay/syncplay/blob/6b490a40980bd61307ea20ede37fd7e4d2c8073e/syncplay/ui/ConfigurationGetter.py#L556-L557 It is def _loadRelativeConfiguration(self): that handles it https://github.com/Syncplay/syncplay/blob/6b490a40980bd61307ea20ede37fd7e4d2c8073e/syncplay/ui/ConfigurationGetter.py#L460-L480 I don't undestand how any URL survives that.

justchokingaround commented 1 year ago

@port19x can you check this please, i haven't used syncplay in a while, and i don't have it installed atm

port19x commented 1 year ago

@port19x can you check this please, i haven't used syncplay in a while, and i don't have it installed atm

I can once I'm home, but better open a separate issue / ping me on discord because I'll probably forget until the end of the workday

pystardust / ani-cli

feat: add support for windows wsl2 #1190

Pull Request Template

Type of change

Description

Checklist

[x] bumped version

[x] all providers return links (not necessarily on a single anime, use debug mode to confirm)

Additional Testcases

Long story short:

`syncplay` treats links as files

cmd call and argument(title) with spaces

Solutions

It should be now ready to review.

pystardust / ani-cli

feat: add support for windows wsl2 #1190

Pull Request Template

Type of change

Description

Checklist

[x] bumped version

[x] all providers return links (not necessarily on a single anime, use debug mode to confirm)

Additional Testcases

Long story short:

syncplay treats links as files

cmd call and argument(title) with spaces

Solutions

It should be now ready to review.

`syncplay` treats links as files