Review extension photo compression pipeline

[jeffbl]

One problem was found in https://github.com/Shared-Reality-Lab/IMAGE-server/issues/457, which was due to compression always running, even on small, small dimensional photos. It also seemed to make small images larger after compression. The quick fix is probably not the correct long-term solution, but should fix things for NFB, and still be better than what we had at CSUN. Things to consider:

1. What cases does this libraray fail in? (when does it make graphics larger?)
2. Why is it so slow (over ~5s on my AMD 3900 workstation!)
3. if there may be a significant delay, should the progress popup appear and say "compressing..." for this step , so that if it is triggered, the user at least knows something is happening?

ping to @JRegimbal for any additional things to consider based on looking at the code this morning.

[JRegimbal]

Looks like we can make the check more robust by just verifying if the file size and image dimensions are within allowed ranges before running compression. As for duration, we might be able to get some benefits by playing with quality but it may be worth looking at the tradeoffs after we remove the majority of cases where compression isn't necessary.

[jeffbl]

One that still has a delay that demonstrates the issue, even after the patch-fix: https://image.a11y.mcgill.ca/pages/imgs/pexels-mikhail-nilov-6981101.jpg

[jeffbl]

@JRegimbal Leaving this in current sprint if you would have time to look at it since you touched on it already. Otherwise will move to next sprint on @jaydeep's plate.

[JRegimbal]

@jeffbl @jaydeepsingh25 do either of you know why we're checking image dimensions in here? Size makes sense since that directly impacts what the server will accept (too large and the request will be refused), but I can't think of a reason why we'd be resizing at this stage.

[jaydeepsingh25]

I think we were doing a similar resizing based on the height/width in a few preprocessors at the server side, may be that is why we tried to implement a similar logic at the extension, my understanding is that Machine Learning models at the server will fail to give correct results if there are too many pixels in an image. Thus we have an additional check for image size to handle the cases where our server side code doesn't handle them and gives wrong results. Please correct me if I am wrong.

[JRegimbal]

I can't speak to the correct/incorrect results...I'd imagine most of these pipelines would handle resizing as part of their basic image preprocessing steps before they get into the CNN (and would probably do so much more quickly than in the browser!) and would otherwise end up throwing a tensor mismatch error or something. I'd imagine @rohanakut knows what the case is with what we're using.

[JRegimbal]

Resizing on the client seems pointless when done solely based on dimensions...in YOLO we resize to 640x640 on the server anyway and in semantic segmentation resizing is also done. Just considering how CNNs work, I have to imagine Azure is doing this behind the scenes as well. Unless @rohanakut has any objections, I'm only going to resize in cases where the size in bytes is above a certain threshold.

[jeffbl]

@rohanakut can you indicate if you see any issues with Juliette's proposal above? If not, we're going to go ahead with merge. @JRegimbal Needs to be stress tested a bit on unicorn before going to pegasus, so please ping me when this gets merged/deployed to unicorn.

[rohanakut]

@JRegimbal I am okay with the proposal. My only concern is that we should not make the images too small. Although big images don't have an impact on the accuracy small images do have an impact. Could you please let me know the threshold that you are planning to set?

[JRegimbal]

@rohanakut > 4MB, the same size threshold already in use in the extension.