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

Send images #69

Open
rachelhamlin opened this issue Dec 4, 2019 · 56 comments
Open

Send images #69

rachelhamlin opened this issue Dec 4, 2019 · 56 comments

Comments

@rachelhamlin
Copy link

rachelhamlin commented Dec 4, 2019

Image Sending Specs

cc @errorists @flexsurfer :)

User Stories

As a user, I want to send media from my device in 1:1, group and public chats.

  • I want to take new photos/videos and send them.
  • I want to upload existing photos/videos from my device and send them.

POC: status-im/status-mobile#9455

  • Relies on IPFS rather than Status IPFS service.
  • Reduce image resolution when uploaded.
  • Camera flow is not complete yet.
  • Users can not enlarge images in chat.
  • Image aspect ratio not maintained in thumbnail.

MVP: Still images only

  • iOS and Android
  • 1:1, group and public chats
  • Still images only
  • As a sending user, I can:
    • Launch camera and take a new image from chat
    • Upload an existing image from my device
    • Note: There is a wait while the image uploads; it appears to be okay if the user navigates away from the chat (e.g. to Stickers tab) while uploading, but should be tested further.
    • Send image OR text; can not couple them in the MVP
  • As a receiving user, I see image thumbnails in their proper aspect ratio within a chat.
    • I can tap on an image for a full screen preview.

Technical Requirements

  • Images will be pinned using Status IPFS service.
  • Images will be read from Cloudflare.
  • Need to decide which resolution to store in order to display different sizes of the image.
    • Possibly upload two versions: one for the thumbnail, and one for the full size view.
    • We'd have to upload the image twice and it would have two different hashes. You would click the thumbnail, see a loader, and then see the full size image.
    • Even small file sizes on our gateway can take 1 minute to upload.
    • If we have 1,000 people uploading images, it could be a problem. Should discuss with Jakub.
  • IPFS hash should be changed to multihash (similar to stickers).

v1, v2: UI enhancements

To be prioritized according to user feedback. Ranked here according to effort/complexity:

Technical Requirements

  • Some of these ideas require interaction with native parts, e.g. downloading and copying, and thus can be quite tricky.

Future versions: Send video and other files

  • Requires more supportive protocol than IPFS
@rachelhamlin
Copy link
Author

@cammellos @yenda @flexsurfer we have these user requirements to build on for image sending.

What's the point of discrepancy with what we already have?

It would be good if we could capture pros/cons for alternative solutions, along with necessary third party services or dependencies.

@cammellos
Copy link
Contributor

I think we floated a few options:

IPFS

IPFS in its current form requires "pinning" in order for the content not to be recycled.
Generally speaking, someone will need to "pin" the content in order for that to be available for a longer period of time (garbage collection is not something we can control, so we don't know how long it will be available for).
Client will access content through IPFS gateways (no different technically from a web server), which they can potentially chose as all the gateways will have access to the same content.

Link sharing

This is just normal image handling, you upload a link to any image on any domain (ipfs gateways included), and you display it to the user.

Sending content through waku

This would work just the same as how messages are currently exchanged, sent through waku likely through a sort of multi-part protocol.

Other options might be possible.

What I think are the pros and cons of each:

IPFS

Pro

  • Potentially decentralized (it would not be in the shape we'd use it)
  • Content-hash out of the box
  • User can potentially chose IPFS gateway where they want to download content from

Cons

  • Pinning issue needs solving (content might get garbage collected at any point)
  • IPFS gateways are not any more privacy preserving than any other web server
  • Image upload/sharing is limited to IPFS (you can't share images from other domains, you'd have to download re-upload)

Link sharing

Pro

  • Large selection of images (anyone can share a link)
  • Any user can upload an image anywhere

Cons

  • Content is available only to a single URL (no choice of web server, no redundancy etc)
  • Web servers might be malicious and track information (IPFS gateways also could, but the selection is much smaller)
  • Content availability is dictated by the server hosting the image

Sending content through waku

Pro

  • Privacy preserving
  • Adds a feature to our protocol (multipart) than can be used for other purposes

Cons

  • Adds load to the whole network (although with waku that should be greatly reduced)

Conclusions

I personally would go for a mix of all of them, basically how overall I would see it working is;

  1. User can send links to images. this will take the form of say:
    {:url "https://blah", :ipfs-hash "hash"}
    On receiving one of this messages the client will decided whether to display the message based on their preferences. By default no images will be allowed, and the user can decide to whitelist/blacklist some domains (similarly to adblock say) or use a different ipfs gateway if the link is an ipfs.

  2. If user "uploads" an image it will be sent through waku through multipart messages, these images will be displayed by default in the client, alternatively could be uploaded to ipfs.

What I don't know is how much impact will be to send images through waku, I don't currently see a big problem with it (we can send them in an efficient format, png for example).

I don't see the advantage of using IPFS exclusively, in terms of privacy is only better if we actually expect users to have their own "trusted" gateways, which I don't believe user have or do, as IPFS gateways are technologically identical to web servers so assuming that are more secure gives users a false sense of security.
We could still use it, just as another option or the default option for uploading ("upload to ipfs or send directly"), in case waku sending is not viable, but limiting to it seems to me that it's not doing us any favour.

@yenda
Copy link
Contributor

yenda commented Apr 20, 2020

I think the profile image feature is related to this one. We will have to decide how to store the image in a way that doesn't leak user information. I don't think it would be desirable if by resolving ens names and profile image the user ip is revealed to a third party for instance. Otherwise that would be too easy to get someone's ip by sending them a message with an account that has an ens name.

@errorists
Copy link

(we can send them in an efficient format, png for example).

in a separate discussion we're thinking of adopting webp image compression for use with other image assets. With a heavier than average compression I'm thinking we could go down to about ~500kb per image uploaded. That's considerably less than the 2mb / image of the usual suspects. Another thing, not sure if it applies here, typically apps store multiple versions of the same image, a low-res thumbnail minuscule in size shown in chat and the full res image that only gets downloaded once the user specifically requests it, usually when opening the full screen preview

@cammellos
Copy link
Contributor

webp sounds like a good idea.

There's also to consider encryption, we can't probably ask the user to upload an image to IPFS without encryption (for one-to-one or group chats for example).
Sending through waku will not have this problem as they'd be already encrypted.

Another thing, not sure if it applies here, typically apps store multiple versions of the same image, a low-res thumbnail minuscule in size shown in chat and the full res image that only gets downloaded once the user specifically requests it,

This should be possible in case we host images somewhere else (so the waku message might contain a thumbnail).
If we go for a waku-only solution then it would not save any bandwidth (as you'd have to send the whole picture anyway).

@errorists
Copy link

just for the record I really like the Waku only solution, that's why I floated the numbers, I could help with providing some data on average image sizes if we're thinking of running a simulation.

@cammellos
Copy link
Contributor

please, I really have no idea myself :) we can put a cap as well

@errorists
Copy link

errorists commented Apr 21, 2020

so there are two things we can do to decrease image file size, one is size, we could downscale images so their longer side is no greater than 2000px. Typically that value is around 4000px for images captured with phone camera or stored in your gallery. The other is compression.
Combined, downscaling with webp compression at 60% quality, I could get JPGs that ranged from 100kb in size (a badly focused picture) to 700kb (a lush landscape with lots of colour data).
Screenshot 2020-04-21 at 09 14 34

edit: I'm also attaching how the same would look with regular JPG 60% compression which is faster computation wise and afaik wouldn't require additional frameworks

Screenshot 2020-04-21 at 09 29 30

@flexsurfer
Copy link
Member

flexsurfer commented Apr 29, 2020

waku and webp looks really interesting especially for 1-1 and group chats
so we could just send markdown something like ![Hello World](data:image/webp;base64,iVBORw0K.. and probably we could decrease image dimensions if needed to keep it under 200kb

@cammellos
Copy link
Contributor

Yes, to me that sounds the most obvious first step, it has low security implications, and should work out mostly out of the box, I would give this a try first.
I think @errorists is not too keen on having images in markdown (correct me if I am wrong), he prefers them to be sent/displayed separately, but in terms of protocol etc, that's just a detail.

@flexsurfer
Copy link
Member

the pros of sending as markdown we don't need any implementation on protocol level then, and users could use this feature without uploading images in status, they could just send strings, not sure if we have limits for message content probably we should have so users don't send huge images at least from the status app

@errorists
Copy link

would markdown mean that in image is placed in the regular message container, just instead of text? That's something I wouldn't like us to do, I'd prefer if the image filled the entire message bubble, without the extra padding text has.

Frame 15

@flexsurfer
Copy link
Member

flexsurfer commented Apr 29, 2020

yeah we could do the same as we do (or did?) it for emoji, if there is only one image in message show it without bubble

@cammellos
Copy link
Contributor

Say we embed them in markdown, how would you handle displaying (I don't mind either approaches myself):
Hi this is ![me](data:image/webp;base64,iVBORw0K....
?

@cammellos
Copy link
Contributor

cammellos commented Apr 29, 2020

Another issue is that for older clients this will be splat out , as they don't have a concept of images:
Hi this is ![me](data:image/webp;base64,iVBORw0K....
They will literally see ![me](data...) which is a bit inconvenient, as opposed to see something like "content-type x" not handled.
Although we could mark the content-type as image as long as there's one image inside, in which case you won't see text nor image.

@errorists
Copy link

errorists commented Apr 29, 2020

regarding markdown, will we be able to know if a message is an image for the purposes of copying pasting, long-press selection, replies etc.? for example if we were to attach individual actions for image type messages? Also for previewing, it's critical we add the tap to preview in full screen view to get the full UX you'd come to expect

@cammellos
Copy link
Contributor

Generally I would say yes, in some implementations it might be slightly more involved, but it should be fine in most cases, so I'd say yes

@cammellos
Copy link
Contributor

What do we need to get the ball rolling on this one?

It seems to me that there's some consensus in at least trying with images over waku, rather than IPFS, at least to start with.

This would mean for the time being that images need to be below 1MB for now, which seems acceptable.
Probably a bit lower than that, say 750KB, but I can look into exactly. Eventually we can have multipart but it's not necessary to get this out.

Webp seems like the format we'd want to support, although I think we can support pngs as well just as easily.

The points still to clarify are:

  1. Do we want to send them embedded in the markdown (i.e some text ![Hello World](data:image/webp;base64,iVBORw0..) some other text) or as a separate message ?

Embedded in markdown

Pros

  • Allows us to mix text and pictures: "Blah Blah [PICTURE] Blah Blah" in a single message
  • Less effort in terms of protcol

Cons

  • More size over the wire (roughly 37%) as base64 encoding is larger
  • We need to handle how existing users will see images (if we don't do anything they will see the literal base64 string, otherwise we can hide the whole message, but in that case you are missing some text as well)

As a separate message type

Pros

  • Less size over the wire, as binary data can be sent
  • Easier to handle compatibility (images will not be shown at all for older clients), or they will be shown "Content-type x not handled", but there won't be any missing text for the user.

Cons

  • Can't mix easily text and images

I probably would favor approach 2 for now, in terms of effort they are basically the same and I think it's a bit more flexible for the time being, although we won't be able to mix text an images (same as whatsapp for example, we can still add a caption of course).

What do you think?
cc @andremedeiros

@arnetheduck
Copy link
Member

Re larger wire size, would it make sense to general-purpose-compress all payloads with something like snappy? This is what's done in eth2 for example

@cammellos
Copy link
Contributor

It would make sense to compress payloads in most cases, as definitely some will benefits from it (images probably won't as they are already highly compressed). The best way to do this is probably to compress at the RLP level, where we can negotiated the protocol between each host, because otherwise we would get into compatibility issues (in public chat we don't know the recipients, so effectively we have to always support non-compressed messages)

@errorists
Copy link

errorists commented Apr 30, 2020

Webp seems like the format we'd want to support, although I think we can support pngs as well just as easily.

just to chime in, WebP is a compression algorithm that can output to any image file container be it a PNG or JPG. The format we want to use is JPG, with or without WebP (we have to test how it performs on mobile, it's noticeably more resource intensive when I used it on an 8-core Mac, compared to regular JPEG compression, it's also not as clear if we'll get smaller file sizes than regular JPEG, see the table.)

@flexsurfer
Copy link
Member

@cammellos
Copy link
Contributor

thanks for the clarification @errorists

@cammellos
Copy link
Contributor

Also, there's a difference on what we send to the wire and what we display.

For example is perfectly ok to send webp on the wire, but have the uncompressed jpg/png displayed to the user (status-go will take care of the conversion), so we don't actually have to support webp in react-native, although we can.

We have quite a lot of freedom here, and we can delay the exact choice of the format once we see how it behaves (changing it it's pretty straightforward),
more important is to decide what to do about embedded vs single message, any opinion?

@errorists
Copy link

errorists commented Apr 30, 2020

Separate message, each image container aka 'message bubble' can only have a single image inside, we shouldn't mix text and images, those would be split into two separate messages on the list : one for image, another for text

@arnetheduck
Copy link
Member

images probably won't as they are already highly compressed

might get some of the base64 losses back though if going with the embedded option - and indeed, doing it at a lower level makes sense - I mentioned mainly in case size was the only blocker against embedding.

@cammellos
Copy link
Contributor

@arnetheduck thanks, it's a good suggestion, we'll see if we can push this forward with waku/1, as bandwidth is an issue.

@errorists thanks, given those consideration is probably safer for now to have a separate content-type for messages, @flexsurfer any objection?

@hesterbruikman
Copy link

hesterbruikman commented Apr 30, 2020

@cammellos with regards to moving this forward. Here are some of the required actions I see. Please tell me if I'm missing any:

  1. Write spec doc based on this thread
  2. Support image message type (what are actions in status-go, status-react and protocol to do this?)
  3. Implement image compression for message type (status-go)
  4. UI implementation of message display

@flexsurfer
Copy link
Member

no objections :)

@flexsurfer
Copy link
Member

  1. finish [WIP] MVP Images status-mobile#9455 UI for capture and image selection

@hesterbruikman
Copy link

To clarify, in what chats can Image Content type be used? Public, 1:1, Group?

@cammellos
Copy link
Contributor

yes, any kind of chat

@hesterbruikman
Copy link

hesterbruikman commented Apr 30, 2020

For reference, these are all related actions:

cc @andremedeiros

@flexsurfer
Copy link
Member

i can take 4-6 if no objections

@hesterbruikman
Copy link

Awesome @flexsurfer, thanks! Any takers for 1? I'm also happy to make a start but in that case I'll still need someone to fill in the blanks

@cammellos
Copy link
Contributor

#69 (comment) should be enough for the technical specs?

@cammellos
Copy link
Contributor

3 is also not needed and won't be done as part of this.

Also, should we capture everything in one tasks with a user story and a clear deliverable? (for example is replying to images necessary to go live or can be done in a second etc) we can split it off in further issues if necessary, but we should encourage working together on this and not each one on separate tasks, as we can get into some issues with integration/discrepancies

@andremedeiros
Copy link

@cammellos can you take that and submit a PR on status-im/specs?

Also, is there a way to do this work in an "adjacent" topic? ie. something users can opt-out or opt-in on?

@hesterbruikman
Copy link

Some more specifics on what happens to images once displayed would be helpful for 7. View images in user profile for 1:1 chats. Are they persistent? If so, how are they organized on the client?

@cammellos
Copy link
Contributor

Are people seeing this :) is the 3rd time I post it:

SPEC CHANGES #107

@hesterbruikman
Copy link

3 is also not needed and won't be done as part of this.

Sorry, the numbers make it look like priorities. Noted, not a requirement. Added the note Optional.

Also, should we capture everything in one tasks with a user story and a clear deliverable? (for example is replying to images necessary to go live or can be done in a second etc) we can split it off in further issues if necessary, but we should encourage working together on this and not each one on separate tasks, as we can get into some issues with integration/discrepancies

I was hoping this could serve as Epic issue. Happy to create one elsewhere if that works better. Agree, added links to next issue and the full list of epic tasks to individual tasks.

Are people seeing this :) is the 3rd time I post it:
SPEC CHANGES #107

That's my bad! Sorry! Was going by the new issue in status-react. Didn't realize how extensive the doc already is

@cammellos
Copy link
Contributor

Also, is there a way to do this work in an "adjacent" topic? ie. something users can opt-out or opt-in on?

Yes, you could post an image on a separate topic and the actual message on the main topic (or post both in the "image topic" but in that way you won't be able to show the user a placeholder for the picture) and only fetch if the user explicitly allow that.

It's a bit cumbersome and you might get into a place where the image was not delivered etc, it would save bandwidth to those users who don't want to fetch images, while slightly increase bw for those who are always opted in

@andremedeiros
Copy link

Awesome, thank you!

I'm obviously assuming this would become an issue (ie. folks wanting to opt out) so I might be making problems up where they don't exist.

If we send images directly now, would that be a "one way decision?" Can that be revisited in the future and re-implemented with the adjacent topic if folks express that opt-out is important to them?

Also, should we capture everything in one tasks with a user story and a clear deliverable?

It sounds like we have functional and technical specs for this now. I'd like to see a list of issues (that reference this one) that take no longer than 3 days to complete. Basically, if we understand the work well enough, we should understand it to the point where we can plan and split up the work.

@cammellos
Copy link
Contributor

If we send images directly now, would that be a "one way decision?" Can that be revisited in the future and re-implemented with the adjacent topic if folks express that opt-out is important to them?

Yes and no :)

So say we go live with this, any client with this version will send images on the main topic, all the clients with this version will be able to receive this.

At some point we want to move to a separate topic.

What will happen is that in newer client everything will work as expected, while older client won't be able to see those images, so we could show some text saying "Upgrade to the latest version to see the image" (which is similar to what we'd show now to existing users once images in any shape or form go live, although they probably will see something like "content-type x not handled", but that we will fix for the next release)

If that's ok then we can do it once there's a clear need.

@hesterbruikman
Copy link

@cammellos @flexsurfer do you see any tasks here that would exceed 3 days to complete? #69 (comment) Disregarding item 3 and 7 for now

@cammellos
Copy link
Contributor

Time estimates again 🤦

3 days is unlikely to be enough for any complex task imo if you take into account testing and QA, and this tasks will likely need manual QA.

4 is definitely more then 3 days to be conservative, disregarding the fact that some code is out already.

@cammellos
Copy link
Contributor

cammellos commented May 6, 2020

Update regarding images:
unfortunately sending them via waku might not be viable, as it takes too long to calculate the PoW for even modest file sizes (256K for example).

Current PoW target is set to 0.002, sending a 290KB image takes around 20 seconds on my laptop (Intel(R) Core(TM) i7-2620M CPU @ 2.70GHz 4 cores). This also means that the message is never propagated as it expires locally.
The good news is that waku/whisper spam protection has some effect :)

I am on a different task this morning, but after that I will re-evaluate, see if there's a potential solution keeping images in waku, otherwise time for plan b.

@flexsurfer
Copy link
Member

what is plan b?

@flexsurfer
Copy link
Member

offtopic: I'm wondering how it worked for avatars, we had pretty huge images for avatars

@cammellos
Copy link
Contributor

We are talking about decreasing the PoW, that's one option. It's a good point for avatars, as a matter of fact I believe many times the image was not sent (it's probabilistic, so you might get lucky every now and then a get it). For example when syncing contacts I had to remove images as they would not be dispatched (I believed at that time it was the size rather than the PoW, but likely it was the PoW instead).

@flexsurfer
Copy link
Member

so if we're going waky way my suggestion is to add image sending support only for 1-1 and group chats for the first time and see how it goes, wdyt @andremedeiros @cammellos @hesterbruikman

@flexsurfer flexsurfer self-assigned this May 11, 2020
@hesterbruikman
Copy link

so if we're going waky way my suggestion is to add image sending support only for 1-1 and group chats for the first time and see how it goes, wdyt @andremedeiros @cammellos @hesterbruikman

Agreed. Less spam sensitive as well. Is it feasible to offer link unfurling for public chats in parallel?

@cammellos
Copy link
Contributor

cammellos commented May 11, 2020

Is it feasible to offer link unfurling for public chats in parallel?

This is a feature to be tracked separately, probably best to have a separate conversation, but it has no dependencies on this work.

@hesterbruikman
Copy link

Url handling in chat epic for future reference: status-im/status-mobile#10516

@flexsurfer flexsurfer removed their assignment Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants