-
Notifications
You must be signed in to change notification settings - Fork 1k
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
[YoutubeBridge] Fix bridge #2208
Conversation
- Add support for custom channel name. - Add new method to fetch JSON data. - Search, listing now parsing data through JSON, HTML is not work anymore. - Remove page number input on the Search endpoint.
This reverts commit 589c918.
- Merge two parseJSON into one. - Remove ytBridgeParseHtmlParsing() due to YouTube not populating HTML in the first run. - Add ability to get better timestamp. - Handle the case of 'Livestream' video.
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.
Empty feed if using search. https://feed.eugenemolotov.ru/pr2208/?action=display&bridge=Youtube&context=Search+result&s=Front+Mission+3&duration_min=&duration_max=&format=Html
Playlist stopped working.
Before PR: https://feed.eugenemolotov.ru/?action=display&bridge=Youtube&context=By+playlist+Id&p=PLJYf0JdTApCqAbZImkQagXEuByh-b_7To&duration_min=&duration_max=&format=Html
After PR: https://feed.eugenemolotov.ru/pr2208/?action=display&bridge=Youtube&context=By+playlist+Id&p=PLJYf0JdTApCqAbZImkQagXEuByh-b_7To&duration_min=&duration_max=&format=Html
bridges/YoutubeBridge.php
Outdated
), | ||
'pa' => array( | ||
'name' => 'page', | ||
'type' => 'number', | ||
'exampleValue' => 1 |
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.
Deleting this parameter will make previous generated feed links invalid. As for now, I suggest to leave it as for now.
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.
@em92 I've pushed new commit that fixed.
- Fix condition error that make some feed stop working. - Fix feed name in the search feed. - Add back the page number parameter on the search feed.
Item timestamps are incorrect. All videos show July 13 (today) with different years. Previous version of timestamp fetching worked fine. Isn't it? |
bridges/YoutubeBridge.php
Outdated
$count++; | ||
if($count == 20) { | ||
break; | ||
} |
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.
Playlist can have videos sorted by ascending (oldest first) upload date. This construction will ignore latest videos, which is not good.
For example: https://www.youtube.com/playlist?list=PLaDrN74SfdT7Ueqtwn_bXo1MuSWT0ji2w
I looked into the video, for example like this: https://www.youtube.com/watch?v=UP091FeB-3o and see the timestamp is 2 years ago 15 minutes after extracted. So that why you see all videos show today with different years. |
I'm going to remove those timestamp and set it back to use the meta timestamp instead. |
…ta, set timestamp back to those in meta tag.
This must be implemented as the other context, not as "User". For example, these channels are different: Everything else looks good and ready to be merged once resolved issue above. |
- Add support for Youtube Data API v3. - Fix playlists that have more than 15 items not display correctly. - Add new custom name parameter. - Fix sorting on playlist not function correctly if use API.
Temporary marking this as draft until PR #2100 is merged |
hi, @csisoap! |
Hi @em92, this PR does supported Youtube Data API v3, which requires an API key. And that PR supported using environment variable for the bridge. I'm intent to add support for getting API key from environment variable once that PR is merged to avoid people accidently commited to the public repo. |
hi again, @csisoap! After you take that API usage out, I will check PR again and, if everything works fine, I will accept it. |
You can merge it now, @em92 |
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.
Following links return empty feed:
https://feed.eugenemolotov.ru/pr2208/?action=display&bridge=Youtube&context=Search+result&s=Front+Mission+3&duration_min=&duration_max=&format=Html (search)
https://feed.eugenemolotov.ru/pr2208/?action=display&bridge=Youtube&context=By+playlist+Id&p=PLaDrN74SfdT7Ueqtwn_bXo1MuSWT0ji2w&duration_min=&duration_max=&format=Html (playlist)
Current version returns non-empty playlist: https://feed.eugenemolotov.ru/?action=display&bridge=Youtube&context=By+playlist+Id&p=PLaDrN74SfdT7Ueqtwn_bXo1MuSWT0ji2w&duration_min=&duration_max=&format=Html
Oops, my bad. Sorry about that. I've pushed a commit that resolve this. |
gj! |
Remove page number input on the Search endpoint.Add ability to get better timestamp.UPDATE 20/07/2021:
Add support for Youtube Data API v3.=> Temporary remove API support, going to create a new PR after this PR merged.Notes: This bridge will handle the case like Video, Livestream. Other videos like Premiering stuff not tested.