Open matrixbot opened 9 years ago
Jira watchers: @Kegsay @ara4n
This is urgent - iOS is uploading images with EXIF that nothing can display.
-- @ara4n
My view: I think we should go with #2, but consider #4.
Rationale on #4:
If the client cannot read EXIF data, then tough. The rationale here is that we cannot realistically enforce that every client submits an orientation
key when uploading, so there may be lightweight clients which just send the image without any additional metadata. It will appear wrong for Little-Client-No-EXIF but display right on any sensible client which parses EXIF information.
Rationale on #2:
This is a slightly more handholdy version than the tough approach on #4. We can handle the rotations server-side and then let Little-Client-No-EXIF download images the Right Way Up. I seriously don't buy the argument that there are clients which do both A: Send things the wrong way around, and B: Cannot correct for this in the EXIF before sending. The added benefit to doing this though is to allow thumbnails to be sensibly downloaded without having to faff with EXIF. We would need to be Very Clear in the spec what w
and h
mean then in this context (pre or post orientation change).
-- @Kegsay
this issue is visible in the riot-web client when uploading images to matrix... it would be nice if a decision was made here so that riot-web can decide whether they need to align on how their android and ios clients work or not.
Just ran across this issue while looking for something else. For whatever it's worth, the decision in matrix-media-repo is to handle orientation when generating thumbnails but to leave the media untouched when downloading.
I can confirm that this is still an issue currently.
This is the way it worked in Riot: When I sent a picture from my PC it had wrong rotation in Riot, since my camera took it with rotation info "right top" in the EXIF data. When I downloaded this image from Riot, it was displayed correcly again in the browser. When I did the same on Riot for Android on my phone, it seems that the image was processed. It had the right orientation but no EXIF data anymore.
As requested in matrix-org/synapse#5039 I add here my opinion.
Current thumbnail implementation for Synapse is inconsitent. Current implementation is generating an scaled image ignoring EXIF orientation and dropping all EXIF information.
This inconsistency can be solved by 2 ways:
Keeping the EXIF information when generating the thumbnail This don't make much sense since usually the first thing that is stripped to reduce the size in one image (to generate a thumbnail) are the EXIF tags. Also the CSS property image-orientation: from-image; is not widely supported.
Rotating the image acording the EXIF orientation tag before dropping all the EXIF information This will make the server behaviour consistent. This don't affect the content of the message for client generated thumbnails:
info.thumbnail_url
, info.thumbnail_info.h
and info.thumbnail_info.w
are filled by the client when the client generate its own thumbnail.info.thumbnail_url
it will call /_matrix/media/r0/download
with the mxc.info.thumbnail_url
should not call /_matrix/media/r0/thumbnail
to retrieve a client generated thumbnail. Only otherwise it could be an issue with info.thumbnail_info.h
and info.thumbnail_info.w
.So I’ve thought about this some more, and conclusion from my side is:
Completely agree with the thumbnail handling.
The orig image should be rendered by clients by considering the JSON data (if present)
This is a shame since it forces every client to have custom Matrix-specific logic in order to display images correctly, rather than a "standard" way by using EXIF. It's also sub-optimal that this metadata doesn't reside with the image data itself, so depending on the client you could have the same image sent with different orientation data, depending on if the client upload supports this JSON data.
Do we actually have any concrete examples of clients that are unable to parse and modify EXIF data? If we don't, then I don't see why the protocol should be contorting itself so much to accommodate the absolute bottom of the barrel.
I'm not extremely fond of overriding EXIF with JSON. If an image has incorrect orientation as per EXIF, maybe fix its EXIF in the first place (on the sending client side, certainly not on the server)?
With my media repo hat on, I think I more agree with the point of servers handling exif when thumbnailing (which isn't that hard: https://github.com/turt2live/matrix-media-repo/blob/37d917976182ea51bc184fa38bd5ec41e9e0d3a6/src/github.com/turt2live/matrix-media-repo/controllers/thumbnail_controller/thumbnail_resource_handler.go#L317-L334 ) and should just return whatever happened to be uploaded (including exif) when /download
is hit.
Clients which want to strip exif for uploads would be responsible for doing that safely by rotating the image appropriately or not stripping that data from the image.
Clients which want to strip exif for uploads would be responsible for doing that safely by rotating the image appropriately
which is precisely what I used to do when I was working on an Android VoIP/IM app prior to Matrix, and that was on incredibly memory constrained hardware (16-24MB heap per application on some of those Android phones...). This is why I want to know if there really are clients out there that can't do this operation if they cannot do EXIF.
Context: Pictures aren't always represented the Right Way Up. It is common to have EXIF data embedded into the file which tells the app/viewer/program the correct orientation of the image, which it then uses to display the image the Right Way Up.
The problem: How do we handle this in Matrix, which is being run on a range of devices (Android, iOS, Web, etc)?
Proposals:
#1 : Matrix
m.room.message
metadata Simply add the correct orientation information to them.room.message
event when sending, along with the existingw
andh
keys.#2 : Homeserver query parameter Make this a homeserver problem and allow Matrix clients to add query parameters like
?right_way_up=true
to both thumbnail and hires Content Repository HTTP GET requests.#3 : Homeserver query parameter with metadata overrides This is identical to #2 but with the ability for the client to specify custom orientation information on upload (e.g. when uploading the file they can add query parameters?)
#4 - Do nothing. Don't handle this at all.
#5 - Server annotates the event. {panel} Uploading client specifies rotation info in the EXIF, fixing the EXIF as required. Uploading HS precomputes thumbnails rotating the image to a display orientation the image based on the EXIF. Uploading client sends an m.room.message event referencing the mxc:// returned by the upload. Uploading HS modifies the event with W+H of the image in the display orientation as it is being sent. Downloading client uses the W+H in the event content to compute the size of the thumbnail it needs. It requests that thumbnail from its HS and assumes that the HS will return the thumnail in the size it requests. Downloading HSes can return the thumbnail size requested, but may choose to return a different size. Downloading HSes should return the thumbnail in the display orientation. Downloading clients can request that their HS returns the image itself in the display orientation. {panel}
#6 - EXIF rewrite extension. Same as option #5 but the uploading server may rewrite the EXIF data with the correct rotation data if the client asks.
#7 - Server returns W+H from the upload. Same as option #5 but the upload server returns the correct W+H to the uploading client. The uploading client uses the W+H to set the W+H in the m.room.message content.
A lot of this boils down to what the lowest common denominator is, and that I think is older versions of Android with buggy cameras. These cameras often don't set the EXIF correctly (!) - they either don't say or outright lie. In these cases, you can hardcode lookups from device build to the "Right Way Up". Critically though, Android has supported reading and writing EXIF data since Android 2.0 (!) so there is no reason why clients cannot set the right oritentation in EXIF.
(Imported from https://matrix.org/jira/browse/SPEC-85)
(Reported by @ara4n)