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

[PeerTube] Add support for PeerTube v6 features #1142

Merged
merged 2 commits into from
Mar 29, 2024
Merged

[PeerTube] Add support for PeerTube v6 features #1142

merged 2 commits into from
Mar 29, 2024

Conversation

TobiGr
Copy link
Contributor

@TobiGr TobiGr commented Dec 25, 2023

  • I carefully read the contribution guidelines and agree to them.
  • I have tested the API against NewPipe.
  • I agree to create a pull request for NewPipe as soon as possible to make it compatible with the changed API.

Description

PeerTube 6.0.0 added support for multiple new features which are implemented in this PR:

  • support for stream segments/chapters extraction: implement PeerTubeStreamExtractor.getStreamSegments()
  • support for storyboard / video preview: implement PeerTubeStreamExtractor.getFrames()

Videos for testing

@TobiGr TobiGr added enhancement New feature or request peertube service, https://joinpeertube.org/ labels Dec 25, 2023
@TobiGr TobiGr force-pushed the peertube-v6 branch 3 times, most recently from 01b44a9 to 1330c4b Compare December 26, 2023 15:10
@TobiGr TobiGr marked this pull request as ready for review December 27, 2023 19:00
@AudricV
Copy link
Member

AudricV commented Jan 26, 2024

I don't think you need to do breaking changes. Can't you catch errors when getting thumbnails and chapters like done for description and throwing an ExtractionException wrapping the original one?

Copy link
Contributor

@Profpatsch Profpatsch left a comment

Choose a reason for hiding this comment

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

I tested it against current newpipe with the example videos and it seems to work pretty well.

@Profpatsch
Copy link
Contributor

One bug I found: The timestamps in chapters are missing:

image

@Profpatsch
Copy link
Contributor

It always jumps to the beginning when I select a chapter cause the timestamp is 0:00.

Copy link
Contributor

@Profpatsch Profpatsch left a comment

Choose a reason for hiding this comment

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

check timestamps

@opusforlife2
Copy link
Collaborator

@Profpatsch Please edit comments to add more info instead of making multiple consecutive comments.

@Profpatsch
Copy link
Contributor

In what way is that relevant?

@TobiGr
Copy link
Contributor Author

TobiGr commented Jan 29, 2024

@Profpatsch Thank you for the review! (I hope that we will be able to review your PRs soon)

Please edit comments to add more info instead of making multiple consecutive comments.

Usually there is a large number of comments on all NewPipe repos, many people get notified about comments and thus receive notifications about new comments. Some team members do community management and therefore check all the new comments on information which might be important for other team members to check out asap. However, if there is no or just little new info in a comment, they waste their time. For that reason we try to motivate people to edit their comments when they want to add minor details.

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Looks good to me. I tested and it works. Thank you!

@Stypox Stypox merged commit fb468a2 into dev Mar 29, 2024
3 of 4 checks passed
@TobiGr TobiGr deleted the peertube-v6 branch March 29, 2024 11:27
@opusforlife2
Copy link
Collaborator

Neither of these features are working for me using the listed test videos on 0.27.0 RC1.

@TobiGr
Copy link
Contributor Author

TobiGr commented Apr 3, 2024

The RC uses @AudricV's yt-fix_comments_extraction branch which is 23 commits behind TeamNewPipe/NewPipeExtractor dev

@opusforlife2
Copy link
Collaborator

Is there a reason it's not up to date? Bit weird to have people test an RC that doesn't have all the features we claim it does.

@Stypox
Copy link
Member

Stypox commented Apr 4, 2024

I guess the reason is that I didn't notice that yt-fix_comments_extraction (i.e. the unmerged #1163) was 23 commits behind dev, otherwise I would have rebased that PR on top of dev.

@TobiGr
Copy link
Contributor Author

TobiGr commented Apr 4, 2024

Could you do that and compile another RC?

@Stypox
Copy link
Member

Stypox commented Apr 4, 2024

Yes, I will for sure for the next RC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request peertube service, https://joinpeertube.org/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants