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

MSC3896: Appservice media #3896

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

Conversation

tezlm
Copy link

@tezlm tezlm commented Sep 24, 2022

@tezlm tezlm changed the title MSCxxxx: Appservice media MSC3896: Appservice media Sep 24, 2022
@turt2live turt2live added proposal A matrix spec change proposal client-server Client-Server API kind:maintenance MSC which clarifies/updates existing spec needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. labels Sep 24, 2022
@rapiz1
Copy link

rapiz1 commented Nov 27, 2022

The duplication of media immediately came to my mind when thinking about building bridges for my use. I joined lots of rooms in an IM service and these rooms may upload various images, files, etc. But I only need a small portion of the files. And for some rooms I don't check often so even images I don't need them. Given that I run matrix on AWS for personal use, it will cause a considerable storage cost. So this is a big blocker for me to adopt matrix and I think this MSC is very useful.

@rapiz1
Copy link

rapiz1 commented Nov 27, 2022

Also, I wonder if it's possible to solve the problem by adapting MSC2246: Asynchronous media uploads.

With MSC2246 implemented, I wonder if it can be done with:

  1. /create a media, giving it an infinite lifetime
  2. retrieve the media and /upload it when a /download request is received

Not familiar with the matrix specs though, just some thoughts

Appservices may set `Cache-Control` on their response. Homeservers should cache the response, though
they may remove cached remote media to save space.

## Potential issues
Copy link
Contributor

Choose a reason for hiding this comment

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

Since currently media ids are largely generated at random, it is very possible that existing media could collide with new namespaced media. Because of this, there needs to be some sort of way to disambiguate this media, either by adding some sort of prefix to media by appservices that homeservers cannot use, vice versa, or something completely different.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not to sure how much of an issue this is, considering:

  • Dendrite uses hex for media IDs ([0-9a-f])
  • Conduit uses alphanumeric chars for media IDs ([0-9a-zA-Z])
  • Synapse uses alphabetical chars ([a-zA-Z])

So perhaps something should be added to the spec stating that for automatically generated media IDs, only alphanumeric characters should be used, so that appservices can use other characters to not collide.

Copy link
Member

Choose a reason for hiding this comment

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

This feels like a similar problem to users: if users already exist under the appservice namespace when the appservice is introduced, they are adopted into the namespace as well. For media, we'd do the same: media which matches the namespace is adopted by the appservice, regardless of when it was created.

The grammar concern is best handled by another MSC (#1597, primarily)

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. I will bring this up in the aforementioned MSC then, thanks!


*none*

## Unstable prefix
Copy link
Contributor

Choose a reason for hiding this comment

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

Should probably specify that for the endpoint, the unstable prefix is /_matrix/app/unstable/org.eu.celery.msc3896/media/{mediaId}

Comment on lines 11 to 12
Whenever the homeserver gets a request that matches the regex, it should make a http GET request
to `/_matrix/app/v1/media/{mediaId}`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would the timeout_ms parameter from the client request be forwarded to the appservice?

Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't really make sense in this context, since this is for async uploads, which doesn't apply for media appservices. Not sure why I even asked this (this can be marked as resolved).

Comment on lines 11 to 12
Whenever the homeserver gets a request that matches the regex, it should make a http GET request
to `/_matrix/app/v1/media/{mediaId}`.
Copy link
Contributor

@Kladki Kladki Apr 16, 2024

Choose a reason for hiding this comment

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

Is the server expected to always follow redirects, or should it use the allow_redirect param like the C-S API?

Copy link
Contributor

Choose a reason for hiding this comment

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

Considering that this was added after the fact to ensure compatibility, I think this should be assumed for new endpoints.


Media may not be able to load if the appservice is unable to reach wherever remote media is stored.

## Alternatives
Copy link
Contributor

@Kladki Kladki May 18, 2024

Choose a reason for hiding this comment

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

mautrix-discord sets up what is essentially a homeserver which only serves media, so perhaps that could be mentioned, and then explain how that is sub-optimal.

In the past it also intercepted requests to your homeserver with regex in a reverse proxy, so that can also be mentioned.

@Kladki Kladki force-pushed the appservice-media branch from 6f29e1f to af67a41 Compare July 1, 2024 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client-server Client-Server API kind:maintenance MSC which clarifies/updates existing spec needs-implementation This MSC does not have a qualifying implementation for the SCT to review. The MSC cannot enter FCP. proposal A matrix spec change proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants