search

LabyStudio / java-spotify-api

Spotify API written in Java to locally access the current song information
15 stars 5 forks source link

Invalid trackID #3

Closed HerXayah closed 1 year ago

HerXayah commented 1 year ago

When using, i can observe an invalid track id. This will cause the following error

java.lang.NullPointerException: Cannot read field "id" because "openTrack" is null
    at de.labystudio.spotifyapi.open.OpenSpotifyAPI.requestOpenTrack(OpenSpotifyAPI.java:247)
    at de.labystudio.spotifyapi.platform.windows.WinSpotifyAPI.onTick(WinSpotifyAPI.java:75)
    at de.labystudio.spotifyapi.platform.AbstractTickSpotifyAPI.onInternalTick(AbstractTickSpotifyAPI.java:69)
    at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539)
    at java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java:305)
    at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:305)
    at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
    at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
    at java.base/java.lang.Thread.run(Thread.java:833)

I attached a screenshot of my IDE below image image

HerXayah commented 1 year ago

Quick Update. This occurs ONLY when using Spotify Installed via scoop.sh. When using normal spotify, itll show this image

NULLYUKI commented 1 year ago

Hello,

I will add some more information regarding this issue. Scoop is installing Spotify from this repository Scoop-Spotify My suspicious what the cause of the issue might be, is that the repository, that scoop is using to download Spotify, does some more modifications to Spotify even though the user wants to install it vanilla without any extra stuff or customization.

I'm currently at my work PC and not at my home PC, so I can't investigate this further. When I'm at my home PC finally, I will investigate what differences are being made between the Spotify installation from the official website and the Spotify installation via Scoop.

I will work on this with @PrincessAkira as we both started to investigate this issue last night, so if one of us find the cause of that issue, we will also try to create a fix for that and create a PR.

As a little extra test, it would be also good to know if this issue also occurs when installing the Spotify package via Chocolatey

NULLYUKI commented 1 year ago

I was able to reproduce this issue by installing Spotify via Scoop.

I used the offical Scoop repository where Spotify is being installed from. Scoop Extras is the repository I used. spotify.json This is the JSON file that Scoop will use to install Spotify I will also add here the content of the JSON file.

spotify.json ```json { "version": "1.2.19.941.gbf202593", "description": "A digital music service that gives you access to millions of songs.", "homepage": "https://www.spotify.com/", "license": { "identifier": "Freeware", "url": "https://www.spotify.com/legal/end-user-agreement/" }, "url": "https://download.scdn.co/SpotifyFullSetup.exe", "hash": "fae73ecaabf126d6dfb23a65741cf35ec9aa75d486fd54dce2d413d1b48874f1", "installer": { "script": [ "Start-Process -Wait \"$dir\\$fname\" -ArgumentList '/extract', \"`\"$dir`\"\"", "# Disable built-in updater", "Remove-Item -ErrorAction Ignore -Recurse \"$env:LOCALAPPDATA\\Spotify\\Update\" | Out-Null", "$updateFile = New-Item -Path \"$env:LOCALAPPDATA\\Spotify\" -Name Update -ItemType File -Value \"Disabled by Scoop\" -Force", "$updatefile.Attributes = 'ReadOnly', 'System'", "Remove-Item \"$dir\\$fname\", \"$dir\\SpotifyMigrator.exe\" -ErrorAction SilentlyContinue" ] }, "shortcuts": [ [ "Spotify.exe", "Spotify" ] ], "uninstaller": { "script": [ "if ($cmd -ne 'uninstall') { return }", "Start-Process -Wait \"$dir\\Spotify.exe\" -ArgumentList '/Uninstall', '/Silent'" ] }, "checkver": { "script": [ "$dl_url = 'https://download.scdn.co/SpotifyFullSetup.exe'", "$dl = cache_path 'spotify' 'unknown' $dl_url", "Invoke-WebRequest $dl_url -OutFile $dl", "$ver = (Get-Item $dl).VersionInfo.ProductVersion", "Move-Item -Force $dl (cache_path 'spotify' $ver $dl_url)", "$ver" ], "regex": "(\\S+)" }, "autoupdate": { "url": "https://download.scdn.co/SpotifyFullSetup.exe" } } ```

Here is also a screenshot on how to install it via Scoop: grafik

NULLYUKI commented 1 year ago

Code Review.

After checking the code via GitHub as I'm at my work PC instead of my home PC right now, I might have found the reason this error occurs.

In the SpotifyProcess class, the method findTrackIdAddress is getting the TrackID by getting the address of it.

    private long findTrackIdAddress() {
        Psapi.ModuleInfo chromeElfModule = this.getModuleInfo("chrome_elf.dll");
        if (chromeElfModule == null) {
            throw new IllegalStateException("Could not find chrome_elf.dll module");
        }

        // Find address of track id (Located in the chrome_elf.dll module)
        long chromeElfAddress = chromeElfModule.getBaseOfDll();
        long addressTrackId = chromeElfAddress + ADDRESS_OFFSET_TRACK_ID;

        if (addressTrackId == -1 || !this.isTrackIdValid(this.readTrackId(addressTrackId))) {
            throw new IllegalStateException("Could not find track id in memory");
        }

        if (DEBUG) {
            System.out.printf(
                    "Found track id address: %s (+%s) [%s%s]%n",
                    Long.toHexString(addressTrackId),
                    Long.toHexString(addressTrackId - chromeElfAddress),
                    PREFIX_SPOTIFY_TRACK,
                    this.readTrackId(addressTrackId)
            );
        }
        return addressTrackId;
    }

Current speculation of the reason that causes this issue.

I'm not sure how different the files gonna be when they are being downloaded via Scoop. So it might get a completely wrong Address that will be used as a TrackID

Possible Solution

The possible solution would be this. The correct address of the TrackID should be accessed via a different method instead of using the chrome_elf.dll

HerXayah commented 1 year ago

In my research of this issue i've also found, that the chrome_elf.dll is significantly smaller than the one from the normal installation of Spotify. image

What you can also observe is, that the Adresses reported by the App on the Scoop Version are too short and hot garbage image

In addition, the song is found in 0x5c3b0000 image

HerXayah commented 1 year ago

HUGE UPDATE

I managed to fix it after getting a new offset image

The problem is, the fix makes the normal version borked haha image

NULLYUKI commented 1 year ago

Good Job on the fix. However this would only work as a temporary solution. Following problems:

HerXayah commented 1 year ago

I am currently working on a fix. I managed to get the PlaybackID etc working, what is missing however, are the timestamps. It can grab the time of the song, however it cant grab the state of the position in the song itself.

The PR can be found here https://github.com/LabyStudio/java-spotify-api/commit/c223c9bf3900ae113303fbc12fcb58751992a58d

NULLYUKI commented 1 year ago

The PR can be found here https://github.com/LabyStudio/java-spotify-api/commit/c223c9bf3900ae113303fbc12fcb58751992a58d

I think you mean commit xd

I am currently working on a fix.

Good that you are working on it, its important that the program itself can always scan for it, so it won't break in future updates.

I managed to get the PlaybackID etc working, what is missing however, are the timestamps. It can grab the time of the song, however it cant grab the state of the position in the song itself.

If I understand the issue you are describing correctly, then I think this has to be done with a check function that will be called every second when a song is playing, so we are not constantly looping it. Here is a little flowchart of what I mean. grafik

HerXayah commented 1 year ago

This already exists. Can be found here https://github.com/LabyStudio/java-spotify-api/blob/e6d23301112174ecdb2b79464f200eb79ff1565e/src/main/java/de/labystudio/spotifyapi/platform/windows/api/playback/MemoryPlaybackAccessor.java#L46

NULLYUKI commented 1 year ago

This already exists. Can be found here

https://github.com/LabyStudio/java-spotify-api/blob/e6d23301112174ecdb2b79464f200eb79ff1565e/src/main/java/de/labystudio/spotifyapi/platform/windows/api/playback/MemoryPlaybackAccessor.java#L46

Ah ok perfect.

It can grab the time of the song, however it cant grab the state of the position in the song itself.

Then I have no idea what you mean with that lmao

NULLYUKI commented 1 year ago

This Fix went more in depth than I ever thought lmao. However I'm glad that I did the code review which made @PrincessAkira researching the issue with the offsets and finding the issue and creating a fix in the PR #4

I hope this will be merged and again. Thanks to @PrincessAkira for the offset research.👍

LabyStudio commented 1 year ago

Good Job on the fix. However this would only work as a temporary solution. Following problems:

  • The offset is hardcoded saved in a variable, which means if the offset might change due Updates it will break. So the offset of the TrackID should be automatically checked to get the correct one and make it work in future versions.
  • To better fix the issue that both version will work, a check is required if the chrome_elf.dll is from the original Spotify version or from the Scoop version.

I had previously implemented a scan but because it did not work reliably on every system (the prefix of the track ID is very often present in the memory) and was also a very heavy operation and took a long time sometimes. That's why I replaced it with a fixed offset which didn't change over many Spotify updates.

I still don't understand how it ran into the NullPointerException even though there is a check for a valid track ID right before

Anyways, the issue should be fixed for now with #4

NULLYUKI commented 1 year ago

I had previously implemented a scan but because it did not work reliably on every system (the prefix of the track ID is very often present in the memory) and was also a very heavy operation and took a long time sometimes. That's why I replaced it with a fixed offset which didn't change over many Spotify updates.

I still don't understand how it ran into the NullPointerException even though there is a check for a valid track ID right before

Anyways, the issue should be fixed for now with #4

Thank you for reading this issue report and merging the PR #4 @PrincessAkira and me, will still in case, keep an eye out, if a future update will break this again and creating a NullPointerException again with the fixed offset.

Should in case a issue like that occur again, or any other issue with the Addon, then we will make sure to quickly check this out and prepare a fix again.

NULLYUKI commented 1 year ago

@LabyStudio

Since this issue has been fixed now, the status of the IDEA IDEA-13066 needs to be updated.

NULLYUKI commented 1 year ago

As a little side note. I installed Spotify via Chocolatey now, to test it with the Spotify Addon. And I can confirm no issues were observed and the Spotify addon was able to get the trackID.

Ok nvm. grafik The Song duration seems broken, but I will investigate that and create a separate issue then.

HerXayah commented 1 year ago

@LabyStudio

Scrap das mit Vanilla und Scoop. Habe noch ein bisschen nachgeforscht und probiert. Scoop installiert die 32Bit Version. Das Offset ist aus der 32Bit Version, weswegen Chocolatey z.B nicht betroffen ist. Das sollte nun jedoch beide, 64 und 32 Bit, abdecken.

Aufjedenfall schönen Abend noch, danke fürs mergen und Sorry für die nochmalige Störung, wollte das nur nochmal klar Stellen, dass das NICHTS mit Scoop zu tun hat.

NULLYUKI commented 1 year ago

Good Morning,

Scoop installiert die 32Bit Version.

This doesn't make sense to me and should be addressed to the Scoop Devs. As 32Bit is already getting much less supported. It just confuses me that Scoop is still downloading a 32Bit Version, instead of a 64Bit version. Which means it still kinda is a Scoop Issue in my opinion. Chocolatey > Scoop I guess lol

Das sollte nun jedoch beide, 64 und 32 Bit, abdecken.

I really like that we made it support 64Bit and 32Bit now. Time to work on MAC and Linux support Would be also a fun challenge in my opinion.

Aufjedenfall schönen Abend noch, danke fürs mergen und Sorry für die nochmalige Störung, wollte das nur nochmal klar Stellen, dass das NICHTS mit Scoop zu tun hat.

I also wish everyone a good evening, thanks to everyone working on this and getting this merged. And now I really need to sleep, I'm drunk and tired af. Goodnight!