-
Notifications
You must be signed in to change notification settings - Fork 399
MSC2448: Using BlurHash as a Placeholder for Matrix Media #2448
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
base: old_master
Are you sure you want to change the base?
Changes from 1 commit
8ba6071
f71535a
8adf2b2
56703fc
793107d
777c30c
b80822e
b240d92
1afad01
60773e4
df5b6d7
571ce2a
e885eae
4b83c51
b893c21
6e8eb59
3695ecf
842c2a0
3994010
616bc81
e0a7442
385be8a
708b756
b761b06
40d71ff
7f13184
1300a6e
2a02d2c
7ea82b4
f93d708
ed9ed5e
48c4d55
fba60db
c837c83
b3f1915
5b8c191
63d4966
676571f
a302197
594bcee
64116ae
9bd0ba6
e7e0fb7
1d954f0
9e981ba
75a4fa6
934b6d7
974d368
5668397
234877c
abf5283
754fa7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -96,6 +96,25 @@ Example response: | |
} | ||
``` | ||
|
||
In addition, a new endpoint will be added to the Media API that allows a | ||
client with an existing mxc URL to retrieve a blurhash for it from the | ||
server. It will take the form of `GET | ||
/_matrix/media/r0/download/{serverName}/{mediaId}/blurhash`. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A different option might be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I kind of like that idea, though it could be a potential exploit vector if there was a vuln in the blurhash library implementation, and maybe a DOS vector if you give it a really big blurhash string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh, ok, #2448 (comment) should've ended up in this thread. Repeating it here instead:
Basically I'm happy to have an endpoint for returning the blurhash string from an mxc URL, though not sure about an image from a blurhash string, due to potential attack vectors and that downloading a blurhash as an image would probably not be much faster than downloading the thumbnail itself. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We've decided to go for the extra endpoint. See #2448 (comment). |
||
|
||
Example request: | ||
|
||
``` | ||
GET /_matrix/media/r0/download/example.com/abcde12345/blurhash | ||
``` | ||
|
||
Example response: | ||
|
||
``` | ||
{ | ||
"blurhash": "LEHV6nWB2yk8pyo" | ||
} | ||
``` | ||
|
||
## Visualisation | ||
|
||
Viewing an image message that is loading: | ||
|
@@ -133,6 +152,10 @@ The `blurhash` key in `m.room.message` should be replaced with | |
`/_matrix/media/unstable/xyz.amorgan/upload`, which keeps the same `blurhash` | ||
response key. | ||
|
||
`/_matrix/media/r0/download/{serverName}/{mediaId}/blurhash` should be | ||
replaced with | ||
`/_matrix/media/unstable/xyz.amorgan/download/{serverName}/{mediaId}/blurhash`. | ||
|
||
The `data-mx-blurhash` attribute in `<img>` tags should be replaced with | ||
`data-xyz-amorgan-blurhash`. | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please not download, as
blurhash
is a valid file name. Ideally this would show up in #2380 not here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yes, poor shape choice.
Would you be happy with a temporary API for this in the meantime until #2380 lands?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about
GET /_matrix/media/r0/blurhash/{serverName}/{mediaId}
instead?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather we pushed 2380 through tbh
All this MSC needs to do is say "also MSC2380, or similar, will have a
blurhash
property`. A dedicated endpoint for such a value is not great.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, yeah that could be useful, though if a client doesn't support it then is having the server render it and then send it down any different from pulling down a thumbnail?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, I misread your comment. So you're suggesting hold off on having the server provide a dedicated blurhash endpoint in favour of MSC2380. That sounds fair, and will just mean clients will need to compute it in implementation for now.
Is that correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, one issue with using MSC2380, and not a dedicated endpoint, is that clients won't be able to specify the X/Y parameters of the resolution of the blurhash (or you could, but adding blurhash-specific parameters to
/info
seems a bit weird).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think not being able to specify the blurhash components is a major issue. You need to know the image anyway, to specify those parameters, so it is probably best if the media repo does that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea the thing generating the blurhash would know the components and the components can be pulled out of the blurhash. I don't think we really need a way to specify the components as that actually adds overhead (the media repo then has to decide if it cares, and possibly recalculate it).
MSC2380 is just to return a bunch of stuff about an uploaded file, such as the blurhash. A separate download endpoint (which takes a blurhash string) could render whatever it was given faster than trying to deal with thumbnailing. Separately to both of those, the upload endpoint could maybe take X and Y components, but I'd personally be inclined to just ignore those in my implementation because uploading something is already hard enough on the server.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so no component specification.
I've updated the doc. Let me know if we're on the same page?