search

status-im / specs

Specifications for Status clients.
https://specs.status.im/
MIT License
14 stars 14 forks source link

Send images #69

Open rachelhamlin opened 4 years ago

rachelhamlin commented 4 years ago

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.

POC: https://github.com/status-im/status-react/pull/9455

MVP: Still images only

Technical Requirements

v1, v2: UI enhancements

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

Technical Requirements

Future versions: Send video and other files

rachelhamlin commented 4 years ago

@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 commented 4 years ago

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

Cons

Link sharing

Pro

Cons

Sending content through waku

Pro

Cons

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 commented 4 years ago

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 commented 4 years ago

(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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

errorists commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

As a separate message type

Pros

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

regarding webp in react native https://github.com/DylanVann/react-native-fast-image/issues/622 https://github.com/DylanVann/react-native-fast-image/issues/560

cammellos commented 4 years ago

thanks for the clarification @errorists

cammellos commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

no objections :)

cammellos commented 4 years ago

@hesterbruikman most of it it's there:

3 is not necessary at the moment, and can be actioned as a separate task

4 is probably message display and sending as well

cammellos commented 4 years ago

spec changes https://github.com/status-im/specs/pull/107

flexsurfer commented 4 years ago
  1. finish https://github.com/status-im/status-react/pull/9455 UI for capture and image selection
hesterbruikman commented 4 years ago

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

cammellos commented 4 years ago

yes, any kind of chat

hesterbruikman commented 4 years ago

For reference, these are all related actions:

cc @andremedeiros

flexsurfer commented 4 years ago

i can take 4-6 if no objections

hesterbruikman commented 4 years ago

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 commented 4 years ago

https://github.com/status-im/specs/issues/69#issuecomment-621748008 should be enough for the technical specs?

cammellos commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

SPEC CHANGES https://github.com/status-im/specs/pull/107

hesterbruikman commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

@cammellos @flexsurfer do you see any tasks here that would exceed 3 days to complete? https://github.com/status-im/specs/issues/69#issuecomment-621782877 Disregarding item 3 and 7 for now

cammellos commented 4 years ago

Time estimates again :facepalm:

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 commented 4 years ago

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 commented 4 years ago

what is plan b?