2015-09-13 11:15 GMT+02:00 Olaf Meeuwissen <paddy-h...@member.fsf.org>: > Matteo Croce writes: > >> 2015-09-12 9:13 GMT+02:00 Olaf Meeuwissen <paddy-h...@member.fsf.org>: >>> Hi Matteo, >>> >>> Thanks for taking the trouble to add optional PNG and JPEG output >>> support! >>> >>> I've done a quick review of both patches and there are a few things that >>> I'd like to point out: >>> >>> - Why do you let users select PNG/JPEG outputs even when the support is >>> not available when HAVE_LIBPNG and/or HAVE_LIBJPEG are undefined? >> >> If the user selects JPEG/PNG when support is not compiled in it gets >> an error message: >> fprintf(stderr, "XXX support not compiled in\n"); > > I saw you added that in the new patches you sent. Wouldn't it make more > sense to catch that during option processing rather than when you start > outputting data?
Right, it would be better >>> - Aren't you leaking memory? The malloc is inside a do-while but the >>> free is on the outside. >> >> the malloc is in "if(first_frame)" so it's called only once per file > > I thought that was the case but wasn't quite sure. I'll pull the code > through valgrind before pushing a proposed upgrade just in case. ok, let me know if you find some leak. >>> - It would be nice to update the scanimage manual page to mention the >>> (potential) support for these image formats. >> >> right, will do in v2 > > One more thing I noticed, you don't special case 1-bit SANE_FRAME_GRAY > scans for JPEG. The JPEG format does not support 1-bit monochrome so > you'll have to do something about that. Your options are refusing to > create an image all together or expanding to 8-bit monochrome. I'll try to expand it to 8 bit gray > Hope this helps, > -- > Olaf Meeuwissen, LPIC-2 FSF Associate Member since 2004-01-27 > Support Free Software Support the Free Software Foundation > https://my.fsf.org/donate https://my.fsf.org/join -- Matteo Croce OpenWrt Developer _______ ________ __ | |.-----.-----.-----.| | | |.----.| |_ | - || _ | -__| || | | || _|| _| |_______|| __|_____|__|__||________||__| |____| |__| W I R E L E S S F R E E D O M ----------------------------------------------------- CHAOS CALMER ----------------------------------------------------- * 1 1/2 oz Gin Shake with a glassful * 1/4 oz Triple Sec of broken ice and pour * 3/4 oz Lime Juice unstrained into a goblet. * 1 1/2 oz Orange Juice * 1 tsp. Grenadine Syrup ----------------------------------------------------- -- sane-devel mailing list: sane-devel@lists.alioth.debian.org http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/sane-devel Unsubscribe: Send mail with subject "unsubscribe your_password" to sane-devel-requ...@lists.alioth.debian.org
