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] added metadata, fix descriptions, fix thumbnail, fix upload date, fix age limit, update tests. #239

Merged
merged 21 commits into from
Feb 8, 2020

Conversation

B0pol
Copy link
Member

@B0pol B0pol commented Jan 20, 2020

Hi,
I fixed some issues with PeerTube that we have now.
Also, I extracted metadata: host, privacy, licence, language, tags.
For YouTube, someone could extract privacy, category (and maybe tags) after this PR, that would be a nice thing.

Fixes TeamNewPipe/NewPipe#2201 (comment) 1. 2. and 5.
3 is already fixed, and I plan to fix 4 (default instance only) and 6 with my future Frontend PR.
TeamNewPipe/NewPipe#2201 (comment) I aimed to do that.

Some in-app before/after screenshots:
Screenshot from PeerTube website
Screenshot from NewPipe (with my changes)

Thumbnail and description before
Thumbnail now
Description now

Btw, for the fronted I already handled the fact we are not getting info:
If no metadata is extracted, it stays as it was before
If only some metadata are extracted, no problem

  • I carefully read the contribution guidelines and agree to them.
  • I did test the API against NewPipe.
  • I agree to ASAP create a PULL request for NewPipe for making in compatible when I changed the api.

thumbnail: quality before: https://peertube.cpy.re/static/thumbnails/d2a5ec78-5f85-4090-8ec5-dc1102e022ea.jpg
quality after: https://peertube.cpy.re/static/previews/d2a5ec78-5f85-4090-8ec5-dc1102e022ea.jpg

description: we were getting about the first 260 characters, we now get full description (with fallback to first 260 chars if the get request for full description fails)
test: updated tests to match description, also changed some test: it was assertEquals(extracted, expected), but the proper way to do it is assertEquals(expected, extracted)

metadata: got host, privacy (public, private, unlisted), licence, language, tags
@TobiGr TobiGr added the peertube service, https://joinpeertube.org/ label Jan 20, 2020
B0pol added 2 commits January 20, 2020 14:36
if the description length is above 254, and ends with ..., it means the description we got from the first request is shortened.
why above 254: because in fact, shortened descriptions length are between 255 : https://framatube.org/videos/watch/24efbe1b-73c0-4d72-a3ff-77c8b32d3fcd
https://framatube.org/videos/watch/1ca99f40-fb5b-4fa4-abe5-4d54325df7fc
and 269: https://framatube.org/videos/watch/4d661d5f-a884-4510-bca8-15cb19aa3fe5

also fixed a typo in StreamExtractor.java
actually, the max description length is 250 after request with our extractor.
during my tests, I made API requests with Firefox, copy/pasted into echo "insert description" | wc, and it was giving a wrong length, maybe due to the escapers, I have no idea
anyway, it's now fixed
@B0pol B0pol changed the title [PeerTube] added metadata, fix descriptions, fix thumbnail, update tests [PeerTube] added metadata, fix descriptions, fix thumbnail, fix upload date, fix age limit, update tests. Jan 23, 2020
B0pol added 2 commits January 23, 2020 19:08
https://peertube.cpy.re/videos/watch/d2a5ec78-5f85-4090-8ec5-dc1102e022ea
anonyme zirbeldrüse is his displayName, what was displayed in NewPipe.
but on the website, it's shown 777@mastodon.xyz
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.

Looks good so far. Just some minor comments.

@B0pol
Copy link
Member Author

B0pol commented Jan 24, 2020

About PR in NewPipe repository: I plan to make a first & quick PR right after this PR get merged, to update extractor version, and fix description formatting, because currently, all descriptions are formatted from HTML, but PeerTube gives plain Markdown, so I will change that, see this commit,
so that all fixes are in NewPipe fast, and a second PR, which will be longer to merge, so that we can discuss about UI, UX…

so that java can automatically translate with Locale.getDisplayLanguage(), instead of always having English name of the language
@B0pol
Copy link
Member Author

B0pol commented Feb 2, 2020

Are there other changes needed?

@TobiGr
Copy link
Contributor

TobiGr commented Feb 2, 2020

Sorry, there are just too many PRs :D I forgot about this one.
Please open the PR in the app repository first so we can assure compatibility when merging this. I'll try to make a final review during the week.

B0pol added a commit to B0pol/NewPipe that referenced this pull request Feb 2, 2020
description:
- PeerTube: it's now full description (it cut at 250 characters before), and it displays ok (newlines are ok, but markdown isn't)
- MediaCCC: descriptions are now displayed well (newlines added)
- YouTube: timestamps in descriptions are clickable and work

more PeerTube fixes:
thumbnail is now high quality
age limit is now handled
upload date in «recently added» feed is good now (it was one hour delayed)
all fixes come from TeamNewPipe/NewPipeExtractor#239, so it need to be merged before this PR
B0pol added a commit to B0pol/NewPipe that referenced this pull request Feb 6, 2020
description:
- PeerTube: it's now full description (it cut at 250 characters before), and it displays ok (newlines are ok, but markdown isn't)
- MediaCCC: descriptions are now displayed well (newlines added)
- YouTube: timestamps in descriptions are clickable and work

more PeerTube fixes:
thumbnail is now high quality
age limit is now handled
upload date in «recently added» feed is good now (it was one hour delayed)
all fixes come from TeamNewPipe/NewPipeExtractor#239, so it need to be merged before this PR
@B0pol
Copy link
Member Author

B0pol commented Feb 8, 2020

Just FYI, I've succeed to extract some metadata for YouTube:

  • Privacy
  • Tags
  • Category

I will extract for YouTube

  • Licence: it won't be hard, I found it in the HTML file.

I won't extract for YouTube:

  • Host: it's always youtube.com
  • SupportInfo: there are no support info in YouTube videos page
  • LanguageInfo: the only way I see is with auto-generated subtitles, but it's not really accurate.

But this will be in another extractor PR.

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.

Thanks again!

@TobiGr TobiGr merged commit 3add704 into TeamNewPipe:dev Feb 8, 2020
@B0pol B0pol deleted the peertube branch February 8, 2020 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
peertube service, https://joinpeertube.org/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants