Conversation
| // 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()); |
There was a problem hiding this comment.
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?
1d917fb to
1ac0656
Compare
|
Chocobozzz (Peertube maintainer) answered this via email:
So if we want to embed Peertube videos, there are two options:
I will go with the second option, as it seems both easier and more useful. |
|
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. |
dc6583f to
6c475c3
Compare
dessalines
left a comment
There was a problem hiding this comment.
Also run clippy, CI failing.
| .videos | ||
| .first() | ||
| .map(|v| Url::parse(&v.url).ok()) | ||
| .flatten(); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
CI is currently broken tho. Maybe you could use rustup to do rustup install 1.59 , and do cargo +1.59 fmt
|
Fixed clippy. For now i dont think we have any breaking changes in main, so lets wait with merging until after 0.16.4 |
|
Hrm needs clippy again. |
6f95b83 to
cccb965
Compare
* Use og:video attribute for embeds, change Post.embed_html to embed_url * fix clippy
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).