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

Embed Peertube videos #2261

Merged
merged 2 commits into from
Jun 2, 2022
Merged

Embed Peertube videos #2261

merged 2 commits into from
Jun 2, 2022

Conversation

Nutomic
Copy link
Member

@Nutomic Nutomic commented May 13, 2022

It seems useful to embed Peertube videos directly in Lemmy, so i gave it a try. Embeds are currently completely disabled on the backend for new posts, but still supported in lemmy-ui so can easily be brought back.

Embeds are currently hidden behind this plus button which is not really obvious. But they do work fine (at least for the current, hardcoded embed url).

Screenshot_20220514_011502

// TODO: need to generate this in a better way. one option is parsing property "og:video:url"
// of the peertube video page for embed url. but as this is federation code, i think
// it would make sense to federate the embed code
embed_html = Some(r#"<iframe src="https://peertube2.cpy.re/videos/embed/0ee82ed7-2582-4851-856d-9b151c3e243c"></iframe>"#.to_string());
Copy link
Member Author

Choose a reason for hiding this comment

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

We need to get the Peertube embed url properly. One option is to parse the site property "og:video:url", but then it will work for all websites, not only Peertube. And as this the federation code, i believe it would make sense to also send the embed url over activitypub. What do you think about this @Chocobozzz?

@Nutomic Nutomic force-pushed the peertube-embed branch 2 times, most recently from 1d917fb to 1ac0656 Compare May 17, 2022 20:11
@Nutomic
Copy link
Member Author

Nutomic commented May 20, 2022

Chocobozzz (Peertube maintainer) answered this via email:

In fact we don't send the embed URL because we use another protocol for that purpose: OEmbed. For example, in "https://peertube2.cpy.re/w/5G8a5viybKjp47xwZ9dY69" HTML we expose this tag: <link rel="alternate" type="application/json+oembed" href="https://peertube2.cpy.re/services/oembed?url=https%3A%2F%2Fpeertube2.cpy.re%2Fw%2F5G8a5viybKjp47xwZ9dY69" title="trailer_1080p" />

Behind "https://peertube2.cpy.re/services/oembed?url=https%3A%2F%2Fpeertube2.cpy.re%2Fw%2F5G8a5viybKjp47xwZ9dY69", there is the iframe embed code:

{
"type": "video",
"version": "1.0",
"html": "<iframe width="560" height="315" sandbox="allow-same-origin allow-scripts allow-popups" title="trailer_1080p" src=["https://peertube2.cpy.re/videos/embed/25feb73e-783d-4c90-998b-d5a81fc1450a\"](https://peertube2.cpy.re/videos/embed/25feb73e-783d-4c90-998b-d5a81fc1450a/) frameborder="0" allowfullscreen></iframe>",
"width": 560,
"height": 315,
"title": "trailer_1080p",
"author_name": "nutomic_channel",
"author_url": "https://peertube2.cpy.re/video-channels/nutomic_channel",
"provider_name": "PeerTube",
"provider_url": "https://peertube2.cpy.re",
"thumbnail_url": "https://peertube2.cpy.re/lazy-static/previews/10d2981a-053f-4d2a-a173-6f95ddd78d88.jpg",
"thumbnail_width": 850,
"thumbnail_height": 480
}

Mastodon supports this protocol, it's the reason why they support PeerTube embeds even if we don't send the embed URL in AP object. You might want to support oembed in Lemmy client to also support other video providers (youtube, vimeo etc).

Otherwise, you can also use opengraph if you prefer since we inject <meta property="og:video:url" content="https://peertube2.cpy.re/videos/embed/25feb73e-783d-4c90-998b-d5a81fc1450a" /> tag in HTML page.

I'm not against to send the embed URL page in AP object, but it's difficult to highlight the difference between the embed and the main page since they have the same mediaType attribute:

{
type: 'Link',
mediaType: 'text/html',
href: 'https://peertube2.cpy.re/videos/watch/25feb73e-783d-4c90-998b-d5a81fc1450a'
}

{
type: 'Link',
mediaType: 'text/html',
href: 'https://peertube2.cpy.re/videos/embed/25feb73e-783d-4c90-998b-d5a81fc1450a'
}

So if we want to embed Peertube videos, there are two options:

  • use oembed, which looks a little more complicated, and would only apply to videos received over activitypub
  • use og:video:url property. this is a bit simpler to implement, and would also work to generate embeds for other websites, such as youtube

I will go with the second option, as it seems both easier and more useful.

@Nutomic
Copy link
Member Author

Nutomic commented May 20, 2022

Updated, this works fine for both Peertube and Youtube in my testing. I changed the field post.embed_html to embed_url, which is more secure as it doesnt allow for arbitrary html, but only iframes. This is a breaking change so should maybe wait with merging.

@Nutomic Nutomic marked this pull request as ready for review May 20, 2022 14:30
@Nutomic Nutomic requested a review from dessalines as a code owner May 20, 2022 14:30
@Nutomic Nutomic force-pushed the peertube-embed branch 3 times, most recently from dc6583f to 6c475c3 Compare May 20, 2022 17:35
Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

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

Also run clippy, CI failing.

.videos
.first()
.map(|v| Url::parse(&v.url).ok())
.flatten();
Copy link
Member

Choose a reason for hiding this comment

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

Since this is finding the first video, maybe the column name should be embed_video_url.

Also I didn't pull down this code to check but hopefully this fails gracefully into a None value.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed the column name. And just tested it with a link that has no video, works fine.

About clippy, unfortunately it doesnt work locally anymore after upgrading to Rust 1.61 because of lint clippy::to_string_in_displayhas been renamed toclippy::recursive_format_impl``. And changing that would break ci i believe.

Copy link
Member

Choose a reason for hiding this comment

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

CI is currently broken tho. Maybe you could use rustup to do rustup install 1.59 , and do cargo +1.59 fmt

@Nutomic
Copy link
Member Author

Nutomic commented May 25, 2022

Fixed clippy. For now i dont think we have any breaking changes in main, so lets wait with merging until after 0.16.4

@dessalines
Copy link
Member

Hrm needs clippy again.

@dessalines dessalines enabled auto-merge (squash) June 2, 2022 21:20
@dessalines dessalines merged commit 339eab0 into main Jun 2, 2022
makotech222 pushed a commit to hexbear-collective/lemmy that referenced this pull request Jun 3, 2022
* Use og:video attribute for embeds, change Post.embed_html to embed_url

* fix clippy
Nutomic added a commit to LemmyNet/lemmy-js-client that referenced this pull request Sep 13, 2022
dessalines pushed a commit to LemmyNet/lemmy-js-client that referenced this pull request Sep 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants