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

MSC2700: Thumbnail requirements for the media repo #2700

Closed
wants to merge 3 commits into from
Closed

MSC2700: Thumbnail requirements for the media repo #2700

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c5a909b
Proposal to standardize thumbnailable types
turt2live Jul 28, 2020
fb174f0
assign number
turt2live Jul 28, 2020
60053e3
Mention MSC2705 as the source for returned types
turt2live Jul 28, 2020
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
55 changes: 55 additions & 0 deletions proposals/2700-thumbnail-types.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# MSC2700: Thumbnail requirements for the media repo

Currently what can be thumbnailed is left as an implementation concern and not one that can be
predicted or anticipated by clients. This relatively small proposal aims to fix that.

## Proposal
Copy link
Contributor

Choose a reason for hiding this comment

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

Thoughts about trying to predict supported mimetypes: Can we add a key to /config which includes which mimetypes are supported, much like we do with the max upload size today?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather not tbh, as it means we're in capabilities negotiation territory and less in the realm of spec.


**Note**: This proposal intentionally does not specify what mimetypes to return for thumbnails.
See [MSC2705](https://github.com/matrix-org/matrix-doc/pull/2705) for details on those restrictions.

This proposal does not alter how the thumbnails are generated, just that they can be generated.

Servers MUST be able to generate thumbnails for the following mimetypes:
* `image/png`
* `image/jpeg` (and `image/jpg`)
* `image/gif`

Servers SHOULD be able to generate thumbnails for the following mimetypes:
* `image/svg+xml`
* `video/H264`
* `video/mp4`
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe also add image/webp to SHOULD as it is becoming more and more standard?

Copy link
Member Author

Choose a reason for hiding this comment

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

There doesn't really appear to be a lot of traction around it, at least not enough to warrant an explicit suggestion in the spec.


Servers SHOULD support other mimetypes where possible, such as `application/pdf` and `text/plain`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to clarify: this means that a media repository can still thumbnail any other formats that they want to?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, the minimum is provided above though

Copy link
Member

Choose a reason for hiding this comment

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

The open-ended list means that clients still cannot reliably predict which MIME types a given server can thumbnail, beyond a very limited set.
Can't we just rely on what comes from /sync or /event and move the decision on whether a file has thumbnail or not to the moment of uploading? Basically, the uploading client could instruct the server to automatically generate thumbnail (on top of supplying the mxc: link to the thumbnail, as it's done now) and get a respective message on whether or not this is possible in the response. Downloading clients would just use the event payloads as they can do now to find thumbnail information - except that if there's none, clients SHOULD NOT ask the server for a thumbnail.

Copy link
Member Author

Choose a reason for hiding this comment

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

The thumbnail endpoint exists so clients can populate that information in the event. It's not always possible to do the thumbnailing client side (encrypted media shows the artifacts nicely,), and the client rendering the event might have different resolution requirements anyways.

tbh, another msc should remove the thumbnail info from the event as it provides much less value to clients.

The open-ended support is a requirement, however there's zero reason why we can't specify a limited set for clients to always try for.

Copy link
Member

Choose a reason for hiding this comment

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

The point about different resolutions is quite convincing, thanks for the reminder. But I still struggle to work out a coherent guidance for myself to implement this in Quaternion. Right now I just try to get a thumbnail for any message event with m.image or m.video. This MSC is supposed to change that but how exactly - this escapes me. I certainly don't want to restrain myself to the three required MIME types, but since the list is open-ended, I should - keep doing what I do now in the hope that the server supports more than just basics?

Copy link
Member Author

Choose a reason for hiding this comment

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

well, as mentioned: the MSC needs rework.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps an endpoint could be added where the server reports which mimetypes it can thumbnail? The client could query that on the first time it wants to thumbnail something and then make a decission based on on that

Copy link
Contributor

Choose a reason for hiding this comment

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

@Sorunome such as https://github.com/matrix-org/matrix-doc/pull/2700/files#r467798151 :p

Copy link
Contributor

Choose a reason for hiding this comment

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

We introduced https://matrix.org/docs/spec/client_server/r0.6.0#get-matrix-media-r0-config to be a general key value map of configuration to inform the client before they try a media operation. It feels like including a list of supported mimetypes would be trivial from a spec and impl perspective?

Copy link
Member

Choose a reason for hiding this comment

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

That would work, yes.


In order to better support the server's mission in thumbnailing, clients SHOULD NOT upload
encrypted with a content type that matches the source material. Instead, clients SHOULD use
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
encrypted with a content type that matches the source material. Instead, clients SHOULD use
encrypted files with a content type that matches the source material. Instead, clients SHOULD use

`application/octet-stream` or similar for unidentifiable/encrypted files.

Servers that cannot handle a given media's mimetype should return a `501 M_UNSUPPORTED` error. If
the server encounters an error while trying to thumbnail known media (such as a client mistakenly
uploading an encrypted file as `image/png`), it should continue to return `500 M_UNKNOWN`.

## Potential issues

The formats chosen could be overly restrictive to users, preventing modern formats for files from
gaining popularity. We can fix this by including them in recommendations and requirements.

## Alternatives

We could leave the thumbnail generation up to the server completely, however clients will then be
asking for thumbnails which might not be possible. By specifying a limited set of commonly-used file
types, we allow clients to be able to intelligently get servers to thumbnail media.

## Security considerations

Some media, such as SVGs, are vulnerable to various well-known attacks. These should be avoided by
Copy link
Member

Choose a reason for hiding this comment

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

These should be avoided by server implementations.

Does "These" refer to the media types or to the attacks?

server implementations.
KitsuneRal marked this conversation as resolved.
Show resolved Hide resolved

## Unstable prefix

No unstable prefix is required for this MSC.

## Implementations

This MSC is inherently implemented by Synapse and matrix-media-repo already.