-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
WIP channels fix #1849
WIP channels fix #1849
Conversation
Wouldn't this be blocked by #1848? Before that gets approved this can't (otherwise you need to remove Bump dep.) |
Thanks, will test hopefully tomorrow. |
Live on ytprivate.com |
url = produce_channel_videos_url(ucid, page, auto_generated: auto_generated, sort_by: sort_by, v2: true) | ||
return YT_POOL.client &.get(url) | ||
def get_channel_videos_response(ucid, page = 1, auto_generated = nil, sort_by = "newest", youtubei_browse = true) | ||
if youtubei_browse |
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.
You always have this as true, so probably you can just delete the other branch.
) | ||
else | ||
url = produce_channel_videos_url(ucid, page, auto_generated: auto_generated, sort_by: sort_by, v2: true) | ||
return YT_POOL.client &.get(url) |
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.
It looks like this is exactly the previous channels call unmodified. The shape of this thing is different than the shape of the other branch, in that you have to do response.body["response"] in this case but the other branch doesn't have that.
If you choose not to delete this, then I'm not sure it make sense to have it in the same method, because it wouldn't be interchangeable at all.
"continuation": continuation, | ||
}.to_json | ||
return YT_POOL.client &.post( | ||
"/youtubei/v1/browse?key=AIzaSyAO_FJ2SlqU8Q4STEHLGCilw_Y9_11qcW8", |
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.
I'm wondering if they ever change this? It's the same for everyone, but maybe they rotate it sometimes.
@@ -388,7 +388,7 @@ def fetch_channel_playlists(ucid, author, auto_generated, continuation, sort_by) | |||
return items, continuation | |||
end | |||
|
|||
def produce_channel_videos_url(ucid, page = 1, auto_generated = nil, sort_by = "newest", v2 = false) | |||
def produce_channel_videos_continuation(ucid, page = 1, auto_generated = nil, sort_by = "newest", v2 = false) |
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.
factoring this out probably isn't needed if you delete the youtubei_browse =false path
Note also that long playlists are broken, but I think it can be included in this PR because the continuation request has the same logic (same headers with a continuation token, if I am not wrong). If you want to know more and find a real fix (in Java), see TeamNewPipe/NewPipeExtractor#567. |
In testing on yewtu.be |
Merging since it's a really important fix. |
This wasn't meant to get merged, at least not as is. I stopped working on it as soon as channels worked again. @tenpura-shrimp could you revert and rewrite this please? I left |
@saltycrys I merged it as is because it worked to fix channels, I know it wasn't ideal, but it's still better than not having channels... so it's okay-ish. Obviously it needs to be enhanced. |
This pull request has been automatically locked since there has not been any activity in it in the last 30 days. If you want to tell us about needed or wanted changes or if problems related to this code are discovered, feel free to open an issue or a new pull request. |
This is a preliminary fix for channels. Needs testing and a proper rewrite.