-
-
Notifications
You must be signed in to change notification settings - Fork 324
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change player clients & deobfuscate params with NewPipeExtractor #1774
base: dev
Are you sure you want to change the base?
Conversation
… logged in player requests + don't send login for IOS client
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
14305798 | Triggered | Google API Key | d4f9aad | innertube/src/main/java/com/zionhuang/innertube/models/YouTubeClient.kt | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
Its normal to have error 403? Access denied? |
This can happen for multiple reasons but for most users it shouldn't. Some questions:
|
@th3y your url contains |
I was switching between WEB, WEB_REMIX and got the same result. i could also share a video probably) |
These are known to be more likely to result in 403s. |
yes If it is missing for you please make sure you are testing with new songs which were never loaded before and are not in the db already. If in doubt just reset the app (delete all data or uninstall->reinstall without restoring a backup). |
Http 403/ New yt blocking: A potoken is required in the middle and end of the video. If you want a solution to play songs: Wait until the Invidious API supports the Invidious companion. Support PR for Invidious companion: iv-org/invidious#4985 Support for android applications using Invidious APİ : iv-org/invidious-companion#22 Project link: https://github.com/iv-org/invidious-companion |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whilst the implementation is working I feel like we are getting dependent of another library. Furthermore, since the IOS player is working (but not for logged in) this PR (as of now) is only for logged in users? It feels like using a bulldozer to move a small rock, since it's essentially 3 function that (if I understood correctly) NewPipe is getting from the /s/player
YouTube's js scripts, we could maybe find a way to abstract from NewPipe and use only the js since it looks like it's the only solution to counter the YouTube's frequent changes. If we could add this to the WEB_REMIX too it would be great, but when trying to do so while testing this PR, I stumbled about the fact that potoken
is now needed (as @Figim stated)
innertube/src/main/java/com/zionhuang/innertube/models/response/PlayerResponse.kt
Show resolved
Hide resolved
I think this is a good thing here because this way we can share efforts with the NewPipe team.
for now - the deobfuscation stuff is only used for logged in users
This is barely a problem. If you make a release build there is an optimization step which means all code from NewPipeExtractor which InnerTune does not use is removed. The final apk only contains the 3 functions (and the functions they call) which are used from NewPipeExtractor. If we "abstract from NewPipe" this will barely help because it already includes only the minimal code it needs from it.
Sure that would be cool because the WEB_REMIX client does provide premium audio formats (higher quality) which are not available with the WEB_CREATOR client. |
@PSJahn The crashes were likely due to #1223 Can you test again with the new |
Now videos play when you sign in. Plays even if you're not logged in. This is great because it can prevent account http 403 errors. |
How was the login fixed? |
Great. Thanks for testing.
There is an issue with Firebase in the |
@z-huang For your info I had to add FOSS PR builds because multiple users reported crashes with the FULL builds currently. |
Google account login works in the full version. Videos are played with the account |
The app also closes when internet is turned off, defeating the purpose of downloading, but I haven't updated since my last comment, so maybe it has also been fixed by one of the commits? |
Link returns 404 for me |
|
Videos are downloaded and played when logged in. I checked in the full and foss version Does the account affect playback? Or not? |
https://music.youtube.com/watch?v=p4oEtV27jdA This video won't download or play. I tried with the account and it still doesn't work. |
I cleared the app data, now it's playing.🤔 |
Its working now, but for some reason some songs are not being playable (new songs, not stored) |
I've tested the apk and most of the songs work except for a few exceptions. I found a channel where some videos are not playable (with "Unknown Error") by the app: https://www.youtube.com/@ARop007 Starting from the song https://www.youtube.com/watch?v=r3P9vakbsaw onward on its channel, I cannot play them. Also, when I have the channel opened, I cannot load the "Videos" list in the app. When I click it hangs (Screenshot) I had the cache clean and the account logged in. Songs/Videos are playable on newpipe Besides this, I really like the idea of depending on Newpipe's lib, from my user perspective it showed to be really stable and quickly patched during this years. |
I cannot reproduce. I downloaded a song, put the phone in airplane mode and it still opened & played without errors |
Please merge the pr so that we can download it @z-huang |
Please do this:
Clearing the cache does not remove the songs from the database. It still keeps some metadata from before which can cause issue. One example is this: If you really want to reload the songs currently you have to reset the whole app (remove app data or uninstall->reinstall without restoring a backup). I known that is annoying and makes testing harder but unfortunately this is still how this app works (I hope this will change in the future). For that reason @ecomaikgolf also try the steps above (with the resets) and check if the songs/videos now play for you. |
Just an info: the IOS client does not have a global One possible fix to make such metadata consistent might be to make two requests:
But I feel like this should be done in a separate PR after merging this one here. |
ReVanced uses android vr (no auth), android tv, Android vr, ios tv (not stable) client ReVanced/revanced-patches#4180 We can wait for the Http 403 error to be fixed by NewPipe. There is a useful tool: https://github.com/LuanRT/BgUtils |
IOS don't let use low quality audio too. |
The sample https://www.youtube.com/watch?v=r3P9vakbsaw worked in step 3 "Try to play the songs again (without login at this point)" The channel was also loadable and all its videos played without issues.
I just checked my exported backup sqlite and some songs have the metadata broken. Does that mean users won't be able to export/import the backup (as this broken-metadata songs won't be able to play anymore)? |
I guess the songs with broken metadata might be playable again at some point as the urls in the database should expire after some time. However I haven't tested it. But it would be better if someone made a PR which fixes the caching issues like this:
This should fix all the caching issues we currently have. |
Update: The WEB_CREATOR client does still have this field but with different values so everything will be much quieter. We should really switch to using the WEB_REMIX client for metadata and use the other clients only for the streams. |
@z-huang This can be merged now. The remaining todos should be handled afterwards but separate from this PR. |
Should fix #1748 #1781 #1775 #1770 #1758 #1764 #1760 #1757
Changes:
Disadvantages:
TODO - can be done in separate PRs afterwards: