Skip to content
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

Fix YouTube playlists continuations #567

Merged
merged 5 commits into from
Mar 4, 2021

Conversation

XiangRongLin
Copy link
Collaborator

@XiangRongLin XiangRongLin commented Mar 3, 2021

  • I carefully read the contribution guidelines and agree to them.

  • Requires sending a POST request instead of GET.

  • clientName and clientVersion, which were required as headers previously now need to be part of the request payload.

  • continuation id also needs to be part of request body.

  • quick and dirty solution (very dirty)

app-debug.zip

I allowed edits by other maintainers, so just adjust it if there are any bugs

Requires sending a POST request instead of GET.
clientName and clientVersion, which were required as headers previously now need to be part of the request payload.
continuation id also needs to be part of request body.

quick and dirty solution.
@XiangRongLin XiangRongLin added the youtube service, https://www.youtube.com/ label Mar 3, 2021
@XiangRongLin XiangRongLin requested review from TobiGr and Stypox and removed request for TobiGr March 3, 2021 18:52
@XiangRongLin
Copy link
Collaborator Author

Normally i would say: "just look at the tests and you can see that the fix works"

But as you can see, the tests fail, now figure out thats it's not because of me....

@FireMasterK
Copy link
Member

FireMasterK commented Mar 3, 2021

Why are you using the hard-coded key? Iirc there was a method in some class to get the key dynamically 🤔

@XiangRongLin
Copy link
Collaborator Author

Why are you using the hard-coded key? Iirc there was a method in some class to get the key dynamically 🤔

Because it's quick and dirty.
That key was exactly the same in youtube-dl and in the request in my browser, so i didn't bother looking out up more in depth.

Feel free to take this PR as a base and to refine it

@FireMasterK
Copy link
Member

I fixed it in FireMasterK@179015b

You can just add this commit :)

@XiangRongLin
Copy link
Collaborator Author

Lovely how i can just open a PR with your changes in your fork against my own branch in my fork. So i don't have to boot up my PC to get the changes 😄

Copy link
Contributor

@TobiGr TobiGr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. I did not have time to take a look at YouTube's requests, but you did, so that should be fine ;)
The tests pass and the changes work in the app.
Can be merged after addressing the review.

@AudricV
Copy link
Member

AudricV commented Mar 4, 2021

I added the same fix for channels and it also works! See my branch. A debug APK can be downloaded here.

@XiangRongLin XiangRongLin merged commit 9256b3b into TeamNewPipe:dev Mar 4, 2021
@XiangRongLin XiangRongLin deleted the playlist_continuations branch March 4, 2021 18:05
@XiangRongLin
Copy link
Collaborator Author

@TiA4f8R You can open a PR for the channels

@AudricV AudricV mentioned this pull request Mar 4, 2021
2 tasks
@TobiGr TobiGr added the bug Issue is related to a bug label Mar 4, 2021
@AudricV AudricV removed the request for review from Stypox March 5, 2021 14:44
@AudricV AudricV added the ASAP Issue needs to be fixed as soon as possible label Mar 5, 2021
@AudricV AudricV changed the title Playlist continuations Fix YouTube playlists continuations Mar 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ASAP Issue needs to be fixed as soon as possible bug Issue is related to a bug youtube service, https://www.youtube.com/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants