SevenTV / image-processor

The image processor which handles emote uploads
Other
4 stars 1 forks source link

Uploading emotes with a height of 32px or below, results in them being nearest neighbor scaled, and there is seemingly no other way to trigger this scaler #70

Open brian6932 opened 2 years ago

brian6932 commented 2 years ago

When you upload something with a height less than or equal to 28px, a different scaler is used to scale the emote to a height of 112px, this scaler seems to treat pixel art in a more lossless way, seemingly nearest neighbor. Now the problem is that this scaler is only used when the input has a height of 28px, it does not work when the height is any larger, meaning you can only have 28px of detail. Maybe some hard-edge detection, or at the very least a user setting should be added to run this scaler no matter the size of the input.

There are some emotes uploaded that doesn't abide by this: https://7tv.app/emotes/62e5ebe8a1a665fe6efd6178 either way an issue to track for this would be good.

TroyKomodo commented 1 year ago

Hi @brian6932, Please can you provide a few examples of how this issue is effecting an upload. Provide an emote which is poor quality which is effected by the resize so I can understand exactly what you are referring to. The resize algorithm we use is https://docs.opencv.org/3.4/da/d54/group__imgproc__transform.html#gga5bb5a1fea74ea38e1a5445ca803ff121acf959dca2480cc694ca016b81b442ceb This is open to be changed if you think it should be something else.

brian6932 commented 1 year ago

Here's an emote that was nearest neighbor scaled to 128x128, then uploaded: https://7tv.app/emotes/61d0dfa8ce8bd4a59cb8958b

Link from before #74 processing, but newer than first: https://7tv.app/emotes/637e933e4fd6f804904956f5 1x: https://cdn.7tv.app/emote/637e933e4fd6f804904956f5/1x.webp

Link from after #74 processing: https://7tv.app/emotes/637fee88b88c4be9814d975d (I don't really notice a difference) 1x: https://cdn.7tv.app/emote/637fee88b88c4be9814d975d/1x.webp

OG File (scaled to 128px) ![ppSlide](https://user-images.githubusercontent.com/18603393/203648510-4e2f5293-7060-4fd3-bbc5-a5d8a88c8ae2.gif)

Now this is the same emote, nearest neighbor scaled down to 28px (with some hand fixes cause nearest neighbor can overshoot scaling down): https://7tv.app/emotes/63798b896ceb1c632b6cb02d 1x: https://cdn.7tv.app/emote/63798b896ceb1c632b6cb02d/1x.webp

OG File (scaled to 28px) ![ppSlide 28px](https://user-images.githubusercontent.com/18603393/203648990-9fe3fbaa-86a6-493d-ab17-6a5c7f0ba45b.gif)

It would be nice if there was a way to post a emote with greater than 28px of detail, and still have an option to scale with a pixel scaling algorithm like nearest neighbor. You could even take the Twitch route, and give an option for the user to provide their own scaling for each size.

OG File (not uploaded) ![cirnoSpinFast](https://user-images.githubusercontent.com/18603393/203651177-fc711eb4-a750-4d68-b9cf-db22381a45ae.gif)

On emotes with > 28px of quality this simply doesn't work: https://7tv.app/emotes/63608dea18c71299d33335e3

Version from after #74 processing: https://7tv.app/emotes/637fef31907973c23ea41f3d (I don't really notice a difference)

OG File (scaled to 128px) ![cirnoSpinFast 128px](https://user-images.githubusercontent.com/18603393/203652032-86a42b25-afa6-412c-bc37-35ce3d07ec90.gif)

Scaling down to 28px: https://7tv.app/emotes/6379bf45485bf2e404bbf5be

OG File (scaled to 28px) ![cirnoSpinFast 28px](https://user-images.githubusercontent.com/18603393/203651733-77f0b9a3-f693-40cf-98fe-c1a95c998d86.gif)
TroyKomodo commented 1 year ago

@brian6932 I just published a version fix to change how resizing works, can u do the test emotes again and update your comment accordingly

brian6932 commented 1 year ago

done

brian6932 commented 1 year ago

Update, maybe when I uploaded those emotes, the scaler wasn't updated for me, but it seems like it is working for less detailed images, like this one: https://7tv.app/emotes/638379f61b2c99744ac2193d which was uploaded at 104x104 (1-3 frames do seem to be broken on larger scaling tho), but not working for this one: https://7tv.app/emotes/638455fc69622ffdbd98e339 which was uploaded at 128x128.

Side note: It seems like the 28px nearest neighbor scaling to 112px no longer works.

TroyKomodo commented 1 year ago

@brian6932 we dont really resize anything on the source upload so I dont really know how the frames could be broken. I'll look into that. I think the solution to this is simply exposing the resize algorithm to the uploader. However this seems like a very niche. I will do it but I am a bit busy at the moment with some other projects for 7TV. PRs if you are willing will be accepted if not I will do it sometime next month.

TroyKomodo commented 1 year ago

You can try to do the make external function in the c++ folder which should build all the external libs, alternatively you can look at the docker file.

On Mon, Nov 28, 2022 at 9:20 AM Brian @.***> wrote:

PRs if you are willing will be accepted if not I will do it sometime next month.

I tried to build the image-processor @TroyKomodo https://github.com/TroyKomodo, however, since the env vars are not documented, I'm not sure what paths to set for gifski:

https://github.com/SevenTV/image-processor/blob/dev/cpp/cmake/FindGifski.cmake

For the record, I have built gifski with --lib

— Reply to this email directly, view it on GitHub https://github.com/SevenTV/image-processor/issues/70#issuecomment-1329193605, or unsubscribe https://github.com/notifications/unsubscribe-auth/AL3YU5O3LRTJ5MGHQ4OWOR3WKS5R3ANCNFSM6AAAAAAR6C2ZKE . You are receiving this because you were mentioned.Message ID: @.***>

brian6932 commented 1 year ago

I did end up getting that dep working, I just had to add target\release to the prefix path

AnatoleAM commented 1 year ago

It shouldn't be possible to upload below 32px, normally emotes get upscaled. We haven't discussed yet about adding an option to avoid upscaling. There seems to be an issue caused by the latest the commit that makes all emotes not upscale, which is wrong

TroyKomodo commented 1 year ago

^ Clueless, offline discussion its actually correct but anatole wants to add the option to upscale emotes. also unrelated entirely to this issue so i am not sure why he commented it here.