Channels: Fix "Youtube API returned error 400" #5059
Conversation
SamantazFox
force-pushed
the
channels-fix
branch
from
490a4a2 to
82248fa
Compare
|
Videos and livestreams work, but the rest is still broken. No error, though, that's progress! |
|
I can validate this does fix the issue with both subscriptions and livestreams showing now and applying the patch below in issue 4975 which i know is a temp solution makes master branch work again for me. I have a private instance on a residential ip. I appreciate the hard work |
Looks to me like there's some parser work to do. |
@iBicha Yes, I realized that late yesterday, but I had to go to sleep x) |
SamantazFox
force-pushed
the
channels-fix
branch
from
7ac7e8b to
4b9ab0d
Compare
This comment was marked as outdated.
This comment was marked as outdated.
The 'lockupViewModel' structure is used in the channel "podcasts" tab
The 'shortsLockupViewModel' structure is used in the channel "shorts" tab
Fix the "newest" link not being bold when 'sort_by' uses the default value Show 60 videos per page, rather than 30
SamantazFox
force-pushed
the
channels-fix
branch
from
4b9ab0d to
301aeff
Compare
This comment was marked as outdated.
This comment was marked as outdated.
|
@SamantazFox - I also tested your changes and everything seems to be working fine again, thanks so much, keeping fingers crossed this gets merged soon. On a side note - what's the issue with playlists? I haven't noticed any problems. |
|
Here is my problem! |
src/invidious/channels/videos.cr
Outdated
| private def make_initial_videos_ctoken(ucid : String, sort_by = "newest") | ||
| object = { | ||
| "15:embedded" => { | ||
| "2:string" => "\n$00000000-0000-0000-0000-000000000000", |
There was a problem hiding this comment.
Weird, shouldn't this be an embedded with a 1:string in it?
There was a problem hiding this comment.
I have absolutely no idea why this one is different like that, but that's what the official client sends, sooooo....
There was a problem hiding this comment.
Actually this is interesting, they are the same!
the \n is basically a decimal 10, which is 1010 binary. That is a field number 1, and a wire type 2 (length-delimited)
then the $ is a decimal 36, which is exactly the length of 00000000-0000-0000-0000-000000000000.
So both objects end up being encoded into the same data.
I think having it like the other one (embedded object with a subfield number 1) would be better for clarity (and could also allow you to refactor / reuse code)
There was a problem hiding this comment.
Ohh, you're right! That's likely a weird bug of our parser Protodec.
having it like the other one (embedded object with a subfield number 1) would be better for clarity (and could also allow you to refactor / reuse code)
Yeah, that's indeed much less cryptic that way! However, I'll keep the two functions separate to not need an extra parameter and a string interpolation.
|
@syeopite It might be a good idea to add a |
SamantazFox
added
the
in-testing
Change explanation, courtesy of iBicha: The \n is basically a decimal 10, which is 1010 binary. That is a field number 1, and a wire type 2 (length-delimited). Then the $ is a decimal 36, which is exactly the length of 00000000-0000-0000-0000-000000000000. So both objects end up being encoded into the same data.
| # Approximate to one minute, as "shorts" generally don't exceed that. | ||
| # NOTE: The actual duration is not provided by Youtube anymore. | ||
| # TODO: Maybe use -1 as an error value and handle that on the frontend? | ||
| duration = 60_i32 |
There was a problem hiding this comment.
Personally I would prefer using an error value instead of inventing a value so that it can be distinguished from videos with an actual duration. The front end could then just hide the duration when it is not available (in my mind not having the information on the page is better than making up a value).
There was a problem hiding this comment.
I strongly agree on using an error value, but the rest of our frontend is not prepared to handle that. Given the urgency of this patch, I'd prefer to take care of it later.
There was a problem hiding this comment.
The Invidious frontend can't even handle null values most of the time 😅 meaning that we have to provide a value even if it doesn't make sense. There's a lot of technical debt that needs to be resolved eventually...
| # Parses an InnerTube lockupViewModel into a SearchPlaylist. | ||
| # Returns nil when the given object is not a lockupViewModel. | ||
| # | ||
| # This structure is present since November 2024 on the "podcasts" and |
There was a problem hiding this comment.
Just a small nitpick about the dates, the podcasts tab has been like that since at least February 2024 as that is when I created the YouTube.js PR to handle it.
There was a problem hiding this comment.
Ah lol. It seems that nobody uses them, as I couldn't find an issue related to that!
SamantazFox
added
ready
and removed
in-testing
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Closes #4029
Closes #5021
Closes #5029
Requires #5027 to be merged first.
Does NOT address #4477
This PR also adds sort option for shorts.