-
Notifications
You must be signed in to change notification settings - Fork 1
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
Add profile photo #36
Comments
Team CommentsDoubt
Praise
Implementation Considerations
Request
|
Potential ApproachesENSAn ENS approach could take advantage of ENS public resolver get-text-data also see EIP 634. Works well for users with a usable ENS name, but not usable for other users. WakuLocal storage and contact detail sync.
IPFS
We could mix approaches. Perhaps defer to the ENS avatar. |
Notes from yesterday's meeting
|
for your consideration, here you'll find designs for the combination of display names and profile pics, and more specifically what would need to be changed to accommodate those 👉 Figma |
@Samyoul regarding draft specs. I'd be most curious how propagating a display name through the network could work. I don't know how this was handled in the past. A concern I have is that we end up with either:
|
@errorists Thanks for the designs. They look cool. So in chat a display name would look like the following: And ENS name would look like the following: Is it possible to make the ENS name a little more premium looking? Maybe a badge or another colour for the name. Just something that makes that ENS name look like something you would want to spend money on. |
@hesterbruikman thanks for these concerns.
No, the approach would involve the display name being included in the encrypted payload, so it would not be possible to even read the display name unless the message was publicly decryptable.
I think that the only feasible method of broadcasting a user's display name would be to include it with each message they send, ENS names works like this. This would mean that each message would have the display name of the sender attached. If a user changes her display name then the devices receiving new messages from the user would need to perform a backend update on the chat key > display name association or we have a chat where the messages using a particular display name show using that display name. We don't need to have formal contacts, we can just have basic chat key > identity mappings that exist in the device's local storage for each identity that the user has communication with. |
I've completed most of the work on the specification . Could @flexsurfer, @andremedeiros and / or @cammellos give your thoughts on the process proposed in my draft document? |
@Samyoul re: ENS names more premium looking, we could do that if we wanted to, here's an example with an '@' badge, I tried the ENS logo too but at that size it's unrecognisable |
Thanks @Samyoul Potential breaking change
This is a potential breaking change, as we already have a Bandwidth consumptionSending images with each message (actual images, not url/ipfs hash) has a potential large impact on bandwidth. Effectively it would be like any message is an image message. Security consideration of pulling images from IPFS or other domainsI would want a whitelist for this, and not allowed by default, otherwise is trivial to detect a given user IP address, if they are online, etc. Otherwise it would invalidate many of the choices we have made before (waku, images, audio, push notifications). Storing imagesImages should be stored in the database, not on the device filesystem (or at least not in cleartext), but that's easy to change (we do this for normal images and profile pictures, which we had before and never fully removed). Caching imagesIn this case I would assume that a given URL is unique to its content, so I would put the burden on the user changing picture to make sure that is the case, so we don't need to refresh the cache.
This might be a bit tricky, as adding a new field might have some issues as it would invalidate previous caches or we'd have to exclude it, but I think we can just get rid of Mixing of UX and vanity featuresThis is more of a UX point,
(1) can already be overridden by using an ENS name, and will be overridden by using user-set profile names (ENS name are mutable, so it can be exploited) I understand that in the designs we add a public key next to the name, which is good. photo -> no good as I can just set an identicon of someone else In my opinion we should not mix the two, as it looks bad for a security focused messenger, what's for security should be clear it's for security and what's vanity. I think this needs a solution, some that comes to mind:
A mix of the above or other can be possible of course, but the point is that we should be very careful not to mix them. Conclusionsorry for the long post :) I am not opposed to the idea on principle, although I would not implement profile pictures given the tradeoffs if it was my call. Sharing only with contacts is a bit easier and we were doing it already, but given the state of contacts now it would just sink the feature immediately, but would solve the bw problem and at that point we could just pass them along in waku. I think if anything, "I set your picture" fits better the product, but gives a much worse UX admittedly. Whatsapp does "set your own picture", but "I set your name", and I would think that's better, given that verifying identities in our product is already challenging. In any case, if we want to go forward with this, the above is my suggestion on what needs to be addressed, very through specs @Samyoul , that's a definitely something we should take as an example! |
@cammellos thanks for the writeup, really well thought out 🙏 re:
Yes, this was my thinking. There should really only be one source of truth to distinguish unique users and imo public key is the best one we got. 3w random name is as much a burden as it's a feature as we're finding out listening to people using the product. Leaving last call I was under the impression that we'll list those somewhere too and Jonny will share gathered community insights. I'm very much open to ideas how to make this transition better, like the one you shared about generated profile images. |
@errorists my 2p are:
So a post would look something like:
In terms of pk vs 3 random words user name, I don't know what's best, I like pks better, but I am a geek, and def. 3 words names are easier to remember. |
@errorists Yes, I like the "@" badge |
@cammellos thank you very much for your feedback, I'm grateful for your perspective. Potential breaking changeYes, removing
To me this would be the solution. Keep the top level Bandwidth consumption / full image payload attachmentTo be honest with this option I was throwing things against the wall to see what we could make work. I have another idea that uses dedicated identity messages that are not attached to the chat message payload. I suggested it in the original call, but I wanted to keep the identity management inline with pre-existing flows. These messages would be sent to a chat and not be visible, but would be parsed effectively the same as an attached Security consideration of pulling images from IPFS or other domains
This is something that I didn't consider. That's a major issue. Hmmm. I think this is actually a supporting factor for my "other idea". Storing images
Ok, that's fine. Caching images
I actually assumed the opposite that the url for any given image is not reflective of the underlying payload. I could swap out a photo on my server keeping the same url. An example is Gravatar, they use the same url for a centrally managed profile image. The link stays the same but the image may change at any time. But I don't think that using URLs is going to be a viable option, at least not without major security precautions.
In my mind adding a new field would necessitate a contact update anyway. ENS AvatarsENS Avatars are a string, they can be anything. They can be a raw base64 encoded image or a URL, or a completely random string of nonsense. ENS has been floated as a solution for user set profile photos, because ENS is decentralised, but the avatar may not be a decentralised solution is could be a link to https://supershadywebsite.ru/tracking_images/virus4u.png . Identity hashThe Mixing of UX and vanity featuresI basically agree with you on what you say here, I'll defer to @errorists's opinion and judgement with regards what the solution to the presentation is. ConclusionI think IPFS is off the table, I think that URLs (Including those via ENS avatars) are off the table, but I do believe that there is still a path forward using Waku only. Images via URLs could be downloaded on the device of the user that is setting the image (therefore any risk associated with a untrustworth URL is assumed entirely by the user of the URL) and propagated using Waku. Let me rework the specs and I'll propose a more robust solution.
Haha, I was actually happy to see your intimidating wall of text 😄 , I knew I was going to get some schooling. |
@cammellos. @errorists I've completed spec draft v2. The main differences between v1 and v2
Example chat |
I've done a little research on Vector based profile images. We can get very high quality images in pretty small sizes. We can attach things like:
|
Rationale against the use of profile imagesAfter analysis and thorough discussion, the team's consensus is that the use of images is not feasible for profile representation. The main concern is that for the optimum use of profile data propagation the data should be attached to each message the user sends, this creates serious bandwidth concerns. Considered solutions with rejection reasons
Most feasible solutionOf the solutions considered the most feasible is detailed in the draft specs v2. This solution basically involves periodically (every 24 hours) sending profile data for active users of a topic to the topic. This solution reduces major bandwidth consumption concerns at the cost of reducing the availability of profile data. The profile data may or may not be received by all parties in a Waku topic and users of a topic may have inconsistent experiences (such as receiving profile data of other users many hours after first interacting with them, or not at all). Another potential solution is to publish the profile identity data detailed in draft spec v2 to a dedicated chat topic for the chat key. The down side of this solution is that the topic listen limit is only 10,000 and could quickly be overloaded with profile data only topics. Most feasible alternativeProfile avatars can be in the range of 20 to 50 bytes in size and could be reasonably attached to every message sent by the user. Additionally avatar data could be easily stored in ENS avatar fields. |
@Samyoul have you read Jarrad's response here, just asking if maybe we should reevaluate the profile images on waku proposal? |
Thanks @errorists I've now caught up with that discussion. |
I've submitted my proposal to update the Status specs here status-im/specs#150 . Also see the nice view of the proposal if git diffs are not your thing. Because Jarrad has expressed his support of profile images, I've reworked my draft spec into something that I believe is feasible and uses as little extra bandwidth as possible, and can be realistically used in all chat contexts (1-1, group, public etc) You'll notice that in the specs I've left room for avatars to be supported 😎 , even via ENS |
Thanks for adding the Also, what's the current thinking on the connection with display names? Will profile images propagate at the same frequency of profile images? I'd like to prevent scenarios we had in the past where it was unclear what the 'status' of a user was (contact or not) as sometimes profile image with be visible, sometimes not. Lastly, what would be research questions we might need to validate with users?
|
TLDR
Simulating the impact of profile images
We can make some simulations, I'm not aware of any pre-existing ones that we could feed assumptions to. But, if we want to have numbers, I could take some averages of number of users in a chat topic and then calculate the worst case scenario and best case and maybe some kind of random mix. 1-1 chat 1-1 chat would have very low bandwidth usage, maybe about an extra 5kb per identity change. Presuming each user only updates their profile data once a month, that would be 5kb per active 1-1 chat.
Group chat Group chat would have a similarly low bandwidth, maybe about an extra 2.5kb per group member per identity change.
Public chat This is the killer, this one will increase the bandwidth by quite a bit. Roughly 2.5kb per average active member per day per all Pulling numbers from the chats I know about, we have about 10 active public chats, with about an average of about 12 active users. So 10 x 12 x 2.5kb x 30 days = 9mb a month
Let me create a simulation and put in my assumptions, I'll share it here and then we can change numbers and get better output. Profile image propagation
So my specification creates a new concept of a What this means is that the whole identity is broadcast as a single entity, so there is no risk of only getting a display name and no profile image etc. This means you either get all the identity data at the same time or you don't get any identity data, which in my opinion is the best user experience. Research Questions
Not sure I understand this. are we talking about users of other products, such as WhatsApp? Sorry UX research is very alien to me. There are also surveys of the current Status user base to answer my questions above:
|
File SizesI've done some research work on cropping, resizing and compressing image data and I'm able to get pretty consistent results. I've put together a repo dedicated to the findings, please click here to see lots of images https://github.com/status-im/image-resizer (there's even some goats). SummaryThumbnailsIf profile images are 64px x 64px in size we can easily have file sizes comfortably below 2.5kb without major quality loss.
We can even set a goal image size and write an algorithm that will find the correct quality to match the image size. In my experimentation I've been able to select a target file size of 1.5kb without major quality loss in most cases. See the samples. Larger ImagesI've been able to get reasonable quality images of 256px x 256px in size around the 10kb mark.
You'll notice that image quality varies wildly relative to file size at larger image sizes. If we target ~10kb we get the above results, the more complex images become the most noticeably degraded, but not horribly so. I would recommend using the larger image size only for contacts. I don't know what our target size should be for this, but if we use larger payloads (25kb for example) for only contacts we can use 512px by 512px images with reasonable quality for users that wish to see the larger image of their contact's profile image See the samples. My question to you @errorists @hesterbruikman and @cammellos is ... Is the quality of these images good enough? |
Nice! Profile thumbnails should be at 80x80 pixels in size, we display them at 40 points in the UI and that requires at least twice the pixels. Our local assets are stored at x3 times their size in points, but that would likely be overkill for photos. For the larger guys, the highest we go is 120 points for these. so 240x240 pixels will be fine. For compression, I'd limit it at no less than 60% jpg, in general 60-80% is the sweet spot. |
Great, thank you for those numbers. I'll make an update to the prototype code and share the results. |
also a random thought I have, we know those appear as circles in the UI so we likely could crop them as such and get rid of all of the data that's not shown in the UI, just replace it with white pixels or something. maybe it would make a small difference. |
Came for the goats, stayed for the numbers. At face value, I think this is a very reasonable quality. @errorists could you please discuss the dimensions we use on Desktop with Simon? See if that changes anything? Also if we set a maximum, that needs to be known to Desktop so the UI can be adapted to it |
lol, fuck logic amirite? 😅 let's do without cropping then. for compression quality, I think we could also max it at 80%, as at this small size going up will yield increasingly diminishing returns. thanks man, the baby elephant is cute. |
The results for 240px images. Basically the same but the margins of difference are much larger. It think the reason for this counter intuitive result is because the compression algorithms seek to keep the image as close to the original as possible by matching colours, and adding none congruent black shapes makes it harder to do this. Also a circle is an unhelpful shape, it basically intersects with lots of the compression chunks and in most cases increases the work required to compress the image. I tried just adding a black square in the corner and that did consistently and significantly reduce the file size. Not very nice to look at though. See the 240px images
|
Ok, I can write an algorithm that does the following:
Hopefully on average the file size will be below the 2.5kb mark. |
you the man! sgtm, the quality is plenty enough. I'm pumped it's finally happening 🙌 |
File size limit justificationThumbnailThe max size limit of 2.5kb for thumbnails was decided based on the following basis:
Results :
Findings
Profile ImageThe max size limit of 16kb for thumbnails was decided based on the following basis:
Results :
Findings
|
@errorists / @hesterbruikman would you mind updating (or just confirming) the designs on this issue with an MVP that we would be happy to release? (or if you could list the points that still we need to clarify). |
@cammellos sure, the full scope of the interface required to facilitate this is found here. It's quite simple, no need for any MVP. I'll also update the original link in the issue |
actually, now that I think about it, it's not that different to what we had before removing Profile Pictures before v1 😅 if you guys still have that somewhere, it should take you at least halfway there with UI |
Closing as this has been implemented |
#4
Add profile photo
Source: Roadmap planning workshop item status-im/status-react#48 and Discuss
User stories
Use cases
Designs
Progress Summary
The text was updated successfully, but these errors were encountered: