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.

Sign up for GitHub

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

Jump to bottom

Video Embeds #5320

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+26 −9
Open

Video Embeds #5320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
50a68fb
Start progres on video embeds
IsaacMarovitz Sep 13, 2024
6795e01
Should actually work
IsaacMarovitz Sep 14, 2024
858c793
Fix
IsaacMarovitz Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bskyweb/cmd/bskyweb/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,10 @@ func (srv *Server) WebPost(c echo.Context) error {
thumbUrls = append(thumbUrls, postView.Embed.EmbedRecordWithMedia_View.Media.EmbedImages_View.Images[i].Thumb)
}
data["imgThumbUrls"] = thumbUrls
} else if postView.Embed.EmbedVideo_View != nil {
data["width"] = postView.Embed.EmbedVideo_View.AspectRatio.Width
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefix all of these fields consistently, like videoWidth

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 •
edited
Loading

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 •
edited
Loading

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.

Copy link

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

These should definitely use a CDN URL, not a direct link to blob on the PDS. If it needs to just be a thumbnail image instead of the actual video, that is too bad but fine.

cc: @ericvolp12 on thoughts on this specifically.

}
}

Expand Down
6 changes: 3 additions & 3 deletions bskyweb/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.22
toolchain go1.22.4

require (
github.com/bluesky-social/indigo v0.0.0-20240624223826-fd66f93ce141
github.com/bluesky-social/indigo v0.0.0-20240908200721-58e6cb486877
github.com/flosch/pongo2/v6 v6.0.0
github.com/ipfs/go-log v1.0.5
github.com/joho/godotenv v1.5.1
Expand Down Expand Up @@ -81,7 +81,7 @@ require (
github.com/spaolacci/murmur3 v1.1.0 // indirect
github.com/valyala/bytebufferpool v1.0.0 // indirect
github.com/valyala/fasttemplate v1.2.2 // indirect
github.com/whyrusleeping/cbor-gen v0.1.1-0.20240311221002-68b9f235c302 // indirect
github.com/whyrusleeping/cbor-gen v0.1.3-0.20240904181319-8dc02b38228c // indirect
github.com/whyrusleeping/go-did v0.0.0-20230824162731-404d1707d5d6 // indirect
github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect
gitlab.com/yawning/secp256k1-voi v0.0.0-20230925100816-f2616030848b // indirect
Expand All @@ -96,7 +96,7 @@ require (
golang.org/x/crypto v0.21.0 // indirect
golang.org/x/net v0.21.0 // indirect
golang.org/x/sync v0.5.0 // indirect
golang.org/x/sys v0.18.0 // indirect
golang.org/x/sys v0.22.0 // indirect
golang.org/x/text v0.14.0 // indirect
golang.org/x/time v0.3.0 // indirect
golang.org/x/xerrors v0.0.0-20231012003039-104605ab7028 // indirect
Expand Down
12 changes: 6 additions & 6 deletions bskyweb/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03
github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM=
github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw=
github.com/bluesky-social/indigo v0.0.0-20240624223826-fd66f93ce141 h1:F3ZceqS82fFfVV3ychWRLNDRFaFlbOwrPic0qJiUVqw=
github.com/bluesky-social/indigo v0.0.0-20240624223826-fd66f93ce141/go.mod h1:dBIOGhsiK0rgEETnxiGiuEyrSx6DKxEotHIPbiKD6WU=
github.com/bluesky-social/indigo v0.0.0-20240908200721-58e6cb486877 h1:oPM+7BOEEPi0XZ8ucya3wFnpF7pcRHo1TzvgQvCsvsM=
github.com/bluesky-social/indigo v0.0.0-20240908200721-58e6cb486877/go.mod h1:Zx9nSWgd/FxMenkJW07VKnzspxpHBdPrPmS+Fspl2I0=
github.com/carlmjohnson/versioninfo v0.22.5 h1:O00sjOLUAFxYQjlN/bzYTuZiS0y6fWDQjMRvwtKgwwc=
github.com/carlmjohnson/versioninfo v0.22.5/go.mod h1:QT9mph3wcVfISUKd0i9sZfVrPviHuSF+cUtLjm2WSf8=
github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44=
Expand Down Expand Up @@ -208,8 +208,8 @@ github.com/valyala/fasttemplate v1.2.2 h1:lxLXG0uE3Qnshl9QyaK6XJxMXlQZELvChBOCmQ
github.com/valyala/fasttemplate v1.2.2/go.mod h1:KHLXt3tVN2HBp8eijSv/kGJopbvo7S+qRAEEKiv+SiQ=
github.com/warpfork/go-wish v0.0.0-20220906213052-39a1cc7a02d0 h1:GDDkbFiaK8jsSDJfjId/PEGEShv6ugrt4kYsC5UIDaQ=
github.com/warpfork/go-wish v0.0.0-20220906213052-39a1cc7a02d0/go.mod h1:x6AKhvSSexNrVSrViXSHUEbICjmGXhtgABaHIySUSGw=
github.com/whyrusleeping/cbor-gen v0.1.1-0.20240311221002-68b9f235c302 h1:MhInbXe4SzcImAKktUvWBCWZgcw6MYf5NfumTj1BhAw=
github.com/whyrusleeping/cbor-gen v0.1.1-0.20240311221002-68b9f235c302/go.mod h1:pM99HXyEbSQHcosHc0iW7YFmwnscr+t9Te4ibko05so=
github.com/whyrusleeping/cbor-gen v0.1.3-0.20240904181319-8dc02b38228c h1:UsxJNcLPfyLyVaA4iusIrsLAqJn/xh36Qgb8emqtXzk=
github.com/whyrusleeping/cbor-gen v0.1.3-0.20240904181319-8dc02b38228c/go.mod h1:pM99HXyEbSQHcosHc0iW7YFmwnscr+t9Te4ibko05so=
github.com/whyrusleeping/go-did v0.0.0-20230824162731-404d1707d5d6 h1:yJ9/LwIGIk/c0CdoavpC9RNSGSruIspSZtxG3Nnldic=
github.com/whyrusleeping/go-did v0.0.0-20230824162731-404d1707d5d6/go.mod h1:39U9RRVr4CKbXpXYopWn+FSH5s+vWu6+RmguSPWAq5s=
github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 h1:bAn7/zixMGCfxrRTfdpNzjtPYqr8smhKouy9mxVdGPU=
Expand Down Expand Up @@ -297,8 +297,8 @@ golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.11.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI=
golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
Expand Down
13 changes: 13 additions & 0 deletions bskyweb/templates/post.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@

{% block html_head_extra -%}
{%- if postView -%}
{%- if videoUrl %}
<meta property="og:type" content="video.other">
{% else %}
<meta property="og:type" content="article">
{% endif %}
<meta property="profile:username" content="{{ profileView.Handle }}">
{%- if requestURI %}
<meta property="og:url" content="{{ requestURI }}">
Expand All @@ -33,6 +37,15 @@
{# Don't use avatar image in cards; usually looks bad #}
<meta name="twitter:card" content="summary">
{% endif %}
{%- if videoUrl %}
<meta property="og:video:type" content="video/mp4">
<meta property="og:video:height" content="{{ height }}">
<meta property="og:video:width" content="{{ width }}">
<meta property="og:image:height" content="{{ height }}">
<meta property="og:image:width" content="{{ width }}">
Comment on lines +44 to +45
Copy link
Contributor

Choose a reason for hiding this comment

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

why both image and video?

<meta property="og:video:secure_url" content="{{ videoUrl }}">
Copy link
Contributor

Choose a reason for hiding this comment

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

I see secure_url in the OG spec page, but it feels legacy/remnant unless some systems actually provide it or there is clear authoritative guidance that it is still required. We live in an https:// world!

<meta property="og:video:url" content="{{ videoUrl }}">
{% endif %}
<meta name="twitter:label1" content="Posted At">
<meta name="twitter:value1" content="{{ postView.IndexedAt }}">
<meta name="article:published_time" content="{{ postView.IndexedAt }}">
Expand Down
Loading