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

Video Embeds #5320

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

IsaacMarovitz
Copy link

I've started an attempt at allowing video embeds for other platforms such as Discord as requested in #5267. I'm not sure how to properly get the video URL or if that is even available in the atprotocol spec yet so I'm leaving this as a draft for now.

} else if postView.Embed.EmbedVideo_View != nil {
data["width"] = postView.Embed.EmbedVideo_View.AspectRatio.Width
data["height"] = postView.Embed.EmbedVideo_View.AspectRatio.Height
data["videoUrl"] = fmt.Sprintf("https://%s/xrpc/com.atproto.sync.getBlob?did=did:plc:%s&cid=%s", srv.xrpcc.Host, did, postView.Embed.EmbedVideo_View.Cid)
Copy link
Author

Choose a reason for hiding this comment

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

This is rather janky, but it should work, open to more real suggestions

@IsaacMarovitz IsaacMarovitz marked this pull request as ready for review September 14, 2024 21:51
} else if postView.Embed.EmbedVideo_View != nil {
data["width"] = postView.Embed.EmbedVideo_View.AspectRatio.Width
data["height"] = postView.Embed.EmbedVideo_View.AspectRatio.Height
data["videoUrl"] = fmt.Sprintf("https://%s/xrpc/com.atproto.sync.getBlob?did=%s&cid=%s", srv.xrpcc.Host, did, postView.Embed.EmbedVideo_View.Cid)
Copy link
Contributor

@mary-ext mary-ext Sep 16, 2024

Choose a reason for hiding this comment

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

  • xrpcc is instantiated with the AppView URL, and AppView does not store raw blobs, so this doesn't actually work
  • We'd have to resolve the actual PDS that the user actually resides in
  • Problem is, this can incur unexpected egress for third-party PDSes, especially noticaeble as videos in Bluesky app itself is taken care of by the CDN. Might not matter much to PDSes hosted in datacenters, but it's a dealbreaker for PDSes in home servers.

Can og:video:url be pointed to a web page or somewhere? Not sure if pointing to an HLS stream (.m3u8) is possible or not (it probably isn't, because the only browser supporting HLS natively is Safari), not sure how sites like Discord handles this sort of thing.

The easiest alternative right now would be to just show the video thumbnail, it's definitely not great but it's better than nothing.

Copy link
Contributor

@devsnek devsnek Sep 17, 2024

Choose a reason for hiding this comment

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

video urls can technically be any random object of your desire, for example youtube provides <meta property="og:video:url" content="https://www.youtube.com/embed/kq39g2vSsKo">. However most sites will have a whitelist of what providers are allowed to use an html embed, and all others are forced to use a well-known video container/encoding. If there's no such cdn asset available and bluesky relies on m3u8, and this appview doesn't provide a whitelisted player embed like youtube does, a thumbnail is probably the best you can do.

Choose a reason for hiding this comment

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

Since cbsky.app already handles video embeds properly, I wonder how it would react to a video stored on an external PDS (or even a currently-offline PDS). I wonder if it would be considered "sanitary" to link to the blob directly like cbsky does, just fetching it from the PDS directly instead of going through the CDN.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I've mentioned this:

Problem is, this can incur unexpected egress for third-party PDSes, especially noticaeble as videos in Bluesky app itself is taken care of by the CDN. Might not matter much to PDSes hosted in datacenters, but it's a dealbreaker for PDSes in home servers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants