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

Increase image-size constraints (post embeds, user profiles) and served resolutions #502

Closed
pfrazee opened this issue Jan 30, 2023 · 4 comments · Fixed by #503
Closed

Increase image-size constraints (post embeds, user profiles) and served resolutions #502

pfrazee opened this issue Jan 30, 2023 · 4 comments · Fixed by #503

Comments

@pfrazee
Copy link
Collaborator

pfrazee commented Jan 30, 2023

We need to improve the quality of images a smidge more. Here's my proposal:

  • app.bsky.embed.external#external thumb:
    • maxSize 300,000 ➡️ 600,000
    • maxWidth 1000 ➡️ 2000
    • maxHeight 1000 ➡️ 2000
  • app.bsky.embed.images image:
    • maxSize 300,000 ➡️ 600,000
    • maxWidth 1000 ➡️ 2000
    • maxHeight 1000 ➡️ 2000
  • app.bsky.actor.profile avatar:
    • maxSize 300,000 ➡️ 600,000
    • maxWidth 1000 ➡️ 2000
    • maxHeight 1000 ➡️ 2000
  • app.bsky.actor.profile banner:
    • maxSize 500,000 ➡️ 750,000
    • maxWidth 3000 ➡️ 6000
    • maxHeight 1000 ➡️ 2000

This is roughly a 2x of all limits. For the image-proxy serving, we also 2x all the limits and we revert the thumb sizing away from "fit" (as that wrecks the ability to serve wide images).

I suggest if we want to consider any higher limits that we look closely at the costs -- not of image storage or bandwidth, but specifically of transfer latency in the federated network. The reason we're being stingy is to ensure that peoples' feeds load in a timely fashion when the extra hops of federation are introduced.

@dholms
Copy link
Collaborator

dholms commented Jan 30, 2023

I'm wondering if we should go even bigger for embeds? Like up to at least a MB. Just from a quick survey of twitter, they have a lot of images that are greater than 600 kb. As Jake pointed out, these are gonna be insubstantial in comparison to videos.

I expect most images to be served off of a gateway that does some processing, which means that the images only really have to make a couple hops: client -> pds -> BGS (which then serves it off the gateway).

From the gateway, we don't have to serve the full resolution image, but I think we should give the clients some more wiggle room in terms of what they upload.

@pfrazee
Copy link
Collaborator Author

pfrazee commented Jan 30, 2023

I'll let yall make the call here, just want to make sure we get it right. If you're not worried about the latency of 1mb then go for it.

@dholms
Copy link
Collaborator

dholms commented Jan 30, 2023

kk
@pfrazee wanted to check in on

we revert the thumb sizing away from "fit"

what's the desired behavior here for thumbs? They're already set to cover which === fill. Is that right? Or do we want inside which === fit?

@dholms
Copy link
Collaborator

dholms commented Jan 30, 2023

Oh actually I see, that was a recent change from #495 that hasn't made it to prod yet 👌

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

Successfully merging a pull request may close this issue.

2 participants