matiasdelellis / facerecognition

Nextcloud app that implement a basic facial recognition system.
GNU Affero General Public License v3.0
509 stars 46 forks source link

Supported image formats #215

Closed james-cook closed 1 year ago

james-cook commented 4 years ago

I'm attempting to show here which image formats are supported "out of the box" and which not - and perhaps how to add these formats.

I wonder though whether rather than having to process myriad image formats - facerecognition could instead use the preview files (which are all .png I believe). Maybe this is what is already done? This would save time re-scanning (e.g. a large number of .tiff) files and avoid setup complexity.

I noticed a number of "Image not loaded" warnings when running php occ face:background_job To test I added a number of extra image formats to the test suite and get these kind of results: (I converted the Big Bang Theory.jpg using IM to .bmp,.gif,*.tiff etc.)

/tmp/d1/pdlib-min-test-suite # make php-test

Processing file: input/BBT.bmp

Fatal error: Uncaught Exception: bmp load error 6: header too small in /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php:35
Stack trace:
#0 /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php(35): CnnFaceDetection->detect('input/BBT.bmp')
#1 /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php(53): findFaces('input/BBT.bmp')
#2 {main}
:
Processing file: input/BBT.gif

Fatal error: Uncaught Exception: Unable to load image in file input/BBT.gif.
You must #define DLIB_GIF_SUPPORT and link to libgif to read GIF files.

Note that you must cause DLIB_GIF_SUPPORT to be defined for your entire project.
So don't #define it in one file. Instead, use a compiler switch like -DDLIB_GIF_SUPPORT
so it takes effect for your entire application. in /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php:35
Stack trace:
#0 /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php(35): CnnFaceDetection->detect('input/BBT.gif')
#1 /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php(53): findFaces('input/BBT.gif')
#2 {main}
  thrown in /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php on line 35
make: *
:

Processing file: input/BBT.tiff

Fatal error: Uncaught Exception: Unknown image file format: Unable to load image in file input/BBT.tiff in /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php:35
Stack trace:
#0 /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php(35): CnnFaceDetection->detect('input/BBT.tiff')
#1 /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php(53): findFaces('input/BBT.tiff')
#2 {main}
  thrown in /tmp/d1/pdlib-min-test-suite/scripts/face_detect.php on line 35
make: *** [Makefile:27: php-test] Err
:

So, so far:

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/87178116-supported-image-formats?utm_campaign=plugin&utm_content=tracker%2F74944432&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F74944432&utm_medium=issues&utm_source=github).
matiasdelellis commented 4 years ago

Hi @james-cook

The test code does not fully reflect how our application works. Pdlib I can open a couple of iamge formats depending on how you compiled it. However, in the application always open temporary files that are converted to jpeg (This to meet the memory requirements). So, we could theoretically analyze any image that Nextcloud can visualize..

I'm interested in keeping this discussion open, more than anything to add support for HEIF files (My girlfriend has an iphone, and this application is useless for her. haha). and see what format are really useful. :smile:

james-cook commented 4 years ago

Hallo Matias, thanks for your comments. I can see in a "real" run that *.bmp files etc. do not cause a crash and for gifs:

yielding
9/10 - Executing task ImageProcessingTask (Process all images to extract faces)
        NOTE: Starting face recognition. If you experience random crashes after this point, please look FAQ at https://github.com/matiasdelellis/facerecognition/wiki/FAQ
yielding
        Processing image /var/nc-data/admin/files/ddI/test.gif
        Image scaled from 2592x1680 to 2036x1320 (since max image area is 2688000 pixels^2)
        Faces found: 0. Image will be skipped because of the following error: Unable to load image in file /tmp/oc_tmp_pBLACn-.gif.
You must #define DLIB_GIF_SUPPORT and link to libgif to read GIF files.

Note that you must cause DLIB_GIF_SUPPORT to be defined for your entire project.
So don't #define it in one file. Instead, use a compiler switch like -DDLIB_GIF_SUPPORT
so it takes effect for your entire application.
        Exception: Unable to load image in file /tmp/oc_tmp_pBLACn-.gif.
You must #define DLIB_GIF_SUPPORT and link to libgif to read GIF files.

Note that you must cause DLIB_GIF_SUPPORT to be defined for your entire project.
So don't #define it in one file. Instead, use a compiler switch like -DDLIB_GIF_SUPPORT
so it takes effect for your entire application. in /var/www/html/apps/facerecognition/lib/BackgroundJob/Tasks/ImageProcessingTask.php:229
Stack trace:
#0 /var/www/html/apps/facerecognition/lib/BackgroundJob/Tasks/ImageProcessingTask.php(229): CnnFaceDetection->detect('/tmp/oc_tmp_pBL...')
#1 /var/www/html/apps/facerecognition/lib/BackgroundJob/Tasks/ImageProcessingTask.php(171): OCA\FaceRecognition\BackgroundJob\Tasks\ImageProcessingTask->findFaces(Object(CnnFaceDetection), Object(OCA\FaceRecognition\Db\Image))
#2

My general question: does facerecognition really "have" to do any conversions if the preview of the file from nextcloud already exists in the system anyway? i.e. does it need to re-process test.gif above if a preview file test.png already exists somewhere in the file system.

Maybe this is a naive question(!) - but there's already a lot of complexity in getting nextcloud to correctly handle X image formats as it is. And:

TBH I can't quite remember where and how and if preview generates its files - I assume they are stored in a nc-data directory somewhere and not regenerated on the fly.

matiasdelellis commented 4 years ago

Hi James,

I can see in a "real" run that *.bmp files etc. do not cause a crash and for gifs:

You change the code for this ???? :confused:

My general question: does facerecognition really "have" to do any conversions if the preview of the file from nextcloud already exists in the system anyway? i.e. does it need to re-process test.gif above if a preview file test.png already exists somewhere in the file system.

Yes. It is necessary, because nextcloud uses very predefined thumbnail sizes. 32x32, 64x64, 128x128, 256x256, 512x512, 1024x1024, 2048x2048, etc. These grow exponentially, and one size may be too small for the analysis, and one too large. We must be able to control it accurately..

this approach may improve facerecognition processing times(?).

Creating new temporary files is absolutely insignificant for analysis.. :sweat_smile: The 99% of fime or memory usage are consumed by the face search process and the only way to control these is by playing with the image sizes.

in this case no need to recompile dlib/pdlib for GIF (or any other "missing" file format) support

No. Not at all. With the current approach, we can search faces in any file that nextcloud can view. We must always generate jpeg files with an appropriate size, and thus ensure that we can read it.

matiasdelellis commented 4 years ago

Not exactly this issue, but now I have a question that should be resolved when addressing this same problem.

The needed to create temporary files I think is quite clear. No? :smiley:

But why we are using jpeg ??? Obviously for historical reasons .. :sweat_smile: ... but jpeg is a good format for it? The jpeg by definition is a lossy format, and nextcloud saves the jpeg images with a quality of 90%. This can distort the images a lot, (You can check it zooming thumbnails), and by reading these images this of course can affect the entire facial recognition process.

@stalker314314 @james-cook What do you think? I suggest using png. So, the files will be of better quality, also larger, but being temporary that doesn't worry me.

stalker314314 commented 4 years ago

I really doubt you will get any improvement (not significant, but any at all) by moving to png. I even think that JPEG is generally more supported and battle tested format all-around (probably more edge cases and performance is drained from libjpeg than from libpng, and I think even for dlib is jpeg first-class citizen). I really don't see any clear benefit (except improving face detection and generating better 128d vector which I doubt). However, if you feel like moving to .png, why not:)

matiasdelellis commented 4 years ago

Hi, Just comment about this PR here, which summarizes the current situation.

https://github.com/matiasdelellis/facerecognition/pull/240

It will be enabled in a similar way that enabble Previews adding in config.php and array with with the supported mime types:

enabledFaceRecognitionMimetype => array ( 0 => image/png', 1 => 'image/jpeg', 2 => 'image/gif', 3 => 'image/bmp', 4 => 'image/x-ms-bmp', 5 => 'image/x-xbitmap' ),

By default just 'image/jpeg' and 'image/png' (Drop GIF and BMP), and although I thought this opens the door to more formats, we are limited by IImage

See: https://github.com/nextcloud/server/blob/master/lib/private/legacy/image.php#L545

Basically GIF, JPEG, PNG, ¿XBM?, ¿WBMP?, and BMP. So it doesn't contribute much. To add support to other formats we must be inspired by PreviewManager

See: https://github.com/nextcloud/server/tree/master/lib/private/Preview

Note that we can't just use this, because it restricts the size of exponentially sized images (64xANY, 128xANY, 256xANY, 512xANY, 1024xANY).

All this said, it is still an improvement that leaves the configuration defined to future.

:wink:

matiasdelellis commented 1 year ago

Well, Almost 3 years later, we can support almost any file reusing the imaginary service. #616 😅