-
Notifications
You must be signed in to change notification settings - Fork 85
Image validation #397
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
Image validation #397
Conversation
|
Any idea what the performance implications are here? |
|
Not really, but that's something I can test. |
|
You can use benchfella. |
|
sounds good, will do, thanks for the tip! |
|
Apologies for the delay, I've been traveling in Morocco and the rocky internet situation I had made it tough to progress. Really feel for those devs in countries with weak infrastructure. Anyways, here are the results: ## ImageBench
benchmark name iterations average time
get_gif_still_stats 50000 55.53 µs/op
get_png_normal_stats 20000 84.29 µs/op
get_gif_moving_stats 20000 101.25 µs/op
get_jpeg_normal_stats 5000 359.24 µs/op
get_png_large_stats 5000 378.44 µs/op
get_jpeg_large_stats 1000 2445.87 µs/opIn summary, I'd somewhat unscientifically say it should add single digit milliseconds to a POST in a ~99th percentile scenario. Work here: https://github.com/roryqueue/code-corps-image-bench . Please let me know if you see a flaw in this measurement methodology. I basically just copied over the incremental code that will run but I certainly may have missed something. |
|
@roryqueue looks really good! We'll also want to consider bringing this in ASAP and working through how, precisely, to do it for Ember: stavro/arc#125 |
|
Gotcha, yeah that sounds good to me. Been digging through the linked for a bit. Am I off track to think we would actually want to use something like this: https://github.com/benefitcloud/ember-uploader#uploading-to-s3 to upload the image, translating the code in this PR to the Ember app in its entirety, and then just send the s3 image path to the Phoenix app? |
|
Oh we'd probably have to sign the upload with a request to Phoenix before the Ember -> S3 step as well, right? |
|
@roryqueue sorry I fell off the wagon on this one. I'm not quite positive, to be honest. Would love some deeper research into it to know for sure. |
|
On vaca now with limited computer access, but I can look into this next week if needed! |
|
@roryqueue absolutely! Enjoy the vacation. |
|
Closing this because we switched to Cloudinary from using |
NOT READ TO MERGE but progress on #289 ... there doesn't seem to be a great package out there to get height and width etc. for png, gif, and jpeg images without bringing in a big dependency e.g. ImageMagick, so I wrote a lot of it by hand after reading through the code of several packages on Awesome-Elixir. Still needs test coverage, parameter tweaking, error handling, and edge case coverage, but let me know if I'm off track and any steering is needed.