matiasdelellis / facerecognition

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

PHP 8 support of php extension #456

Closed BigMichi1 closed 2 years ago

BigMichi1 commented 3 years ago

Expected behaviour

the php extension that is needed should also be available for PHP 8

Actual behaviour

we are using https://launchpad.net/~ondrej/+archive/ubuntu/php to have a recent up-to-date version of PHP but currently the pdlib extension is not available for PHP 8

Steps to reproduce

  1. use php-8 to serve nextcloud
  2. pdlib extension is not available

Server configuration

matiasdelellis commented 3 years ago

Hi @BigMichi1

pdlib supports php 8 since 1.0.1, but you have to compile it on your own, because I can't make packages for custom repositories.

However, I clarify that it is not yet officially supported by the application. Most likely it will work, but it is not supported yet. See #444

hunterzero99 commented 3 years ago

I'd like to expand this bug report to include errors that seem to be associated with php8. I get the following when trying to run the scan:

Faces found: 0. Image will be skipped because of the following error: Error during image resize

I see mention of this in other bug reports that have been closed out (#429 for example).

Is there any way I can get more debug logging out of the run to see where in the resize process things fail?

gchmurka123 commented 3 years ago

I have the same problem, I am unable to debug why I have the same error: Faces found: 0. Image will be skipped because of the following error: Error during image resize

Kitt3120 commented 3 years ago

Same problem. Using dlib-cuda 19.22-1 and php-pdlib 1.0.2-4 from the AUR with Nextcloud 22.1.1 and php 8.0.10.

Processing image /mnt/Media3/NextcloudData/username/files/Kamera Upload/Pixel 3 XL/2021/08/PXL_20210810_121912205.MP.jpg
        Faces found: 0. Image will be skipped because of the following error: Error during image resize
        RuntimeException: Error during image resize in /var/lib/nextcloud/apps/facerecognition/lib/Helper/TempImage.php:153
Stack trace:
#0 /var/lib/nextcloud/apps/facerecognition/lib/Helper/TempImage.php(125): OCA\FaceRecognition\Helper\TempImage->resizeImage()
#1 /var/lib/nextcloud/apps/facerecognition/lib/Helper/TempImage.php(71): OCA\FaceRecognition\Helper\TempImage->prepareImage()
#2 /var/lib/nextcloud/apps/facerecognition/lib/BackgroundJob/Tasks/ImageProcessingTask.php(203): OCA\FaceRecognition\Helper\TempImage->__construct()
#3 /var/lib/nextcloud/apps/facerecognition/lib/BackgroundJob/Tasks/ImageProcessingTask.php(123): OCA\FaceRecognition\BackgroundJob\Tasks\ImageProcessingTask->getTempImage()
#4 /var/lib/nextcloud/apps/facerecognition/lib/BackgroundJob/BackgroundService.php(120): OCA\FaceRecognition\BackgroundJob\Tasks\ImageProcessingTask->execute()
#5 /var/lib/nextcloud/apps/facerecognition/lib/Command/BackgroundCommand.php(138): OCA\FaceRecognition\BackgroundJob\BackgroundService->execute()
#6 /usr/share/webapps/nextcloud/3rdparty/symfony/console/Command/Command.php(255): OCA\FaceRecognition\Command\BackgroundCommand->execute()
#7 /usr/share/webapps/nextcloud/3rdparty/symfony/console/Application.php(1009): Symfony\Component\Console\Command\Command->run()
#8 /usr/share/webapps/nextcloud/3rdparty/symfony/console/Application.php(273): Symfony\Component\Console\Application->doRunCommand()
#9 /usr/share/webapps/nextcloud/3rdparty/symfony/console/Application.php(149): Symfony\Component\Console\Application->doRun()
#10 /usr/share/webapps/nextcloud/lib/private/Console/Application.php(209): Symfony\Component\Console\Application->run()
#11 /usr/share/webapps/nextcloud/console.php(99): OC\Console\Application->run()
#12 /usr/share/webapps/nextcloud/occ(11): require_once('/usr/share/weba...')
#13 {main}
yielding
matiasdelellis commented 3 years ago

Hi everyone,

As you can see the nextcloud folks are deliberately ignoring PHP 8 tests..

Our application makes intensive use of the OC_Image class, that is not deprecated, far from it, but it seems to fail, and they just don't want to fix it.

Hope to fix it soon, but just I don't recommend PHP 8 yet.. 😞

matiasdelellis commented 3 years ago

Need help.. https://github.com/nextcloud/server/blob/0447b53bda9fe95ea0cbed765aa332584605d652/lib/private/legacy/OC_Image.php#L918-L944

You have to see more info in the nextcloud logger. Please comment what it says ..

guystreeter commented 2 years ago

I don't see any nextcloud log messages associated with the attempt to run the background task. Can you tell us more about how to get the logs you're looking for?

guystreeter commented 2 years ago

I put a lot of logging statements in preciseResizeNew, including this one right before the return:

                if ($process === false) {
                        $this->logger->error('resize: process is false', ['app'  => 'core']);
                } else {
                        $this->logger->error('resize: ' . get_class($process), ['app'  => 'core']);
                }

It always logs:

{"reqId":"l8gbSJ2I4S7QP7W21tZn","level":3,"time":"2021-10-25T21:03:20+00:00","remoteAddr":"","user":"--","app":"core","method":"","url":"--","message":"resize: GdImage","userAgent":"--","version":"22.2.0.2"}

So it looks like preciseResizeNew is not returning false

guystreeter commented 2 years ago

I added this after the check for false:

                if (is_resource($process) === false) {
                        $this->logger->error(__METHOD__ . '() is_resource is false', ['app'  => 'core']);
                }

And it results in

{"reqId":"LnNjBvI6WYUbHLJFgr45","level":3,"time":"2021-10-25T21:12:54+00:00","remoteAddr":"","user":"--","app":"core","method":"","url":"--","message":"OC_Image::preciseResizeNew() is_resource is false","userAgent":"--","version":"22.2.0.2"}

This may be the limit of my understanding of this code. I'll be happy to help if you have more suggestions.

guystreeter commented 2 years ago

is_object($process) is true. var_export($process) shows GdImage::__set_state(array(\n))

guystreeter commented 2 years ago

This article https://php.watch/articles/resource-object talks about the PHP 8 transition from resources to objects. It suggests changing is_resource($x) to $x !== false as a backward-compatible check for successful object creation. Making this change everywhere in OC_Image.php got rid of the error message, but the face thumbnails show up as "broken image" icons in the UI.

guystreeter commented 2 years ago

I apparently had some local problem that was causing the image display issues. They don't happen in a fresh install. With the fixes to OC_Image.php I mentioned previously. the background job runs successfully and clustering works as expected. The only problem I have now is that in the sidebar Persons tab, if there is a recognized person in the photo, I get

Error: Request failed with status code 500

instead of the info about the person. I have no idea how to start debugging that.

matiasdelellis commented 2 years ago

Hi @guystreeter Thank you very much for taking this job!. 😄

Error 500 is an internal error.. Probably in on FaceController.php, and you have to see something in the nextcloud log.. 😬

matiasdelellis commented 2 years ago

Hi @guystreeter Test:

[matias@nube Db]$ git diff .
diff --git a/lib/Db/Face.php b/lib/Db/Face.php
index bbe373c..0699439 100644
--- a/lib/Db/Face.php
+++ b/lib/Db/Face.php
@@ -197,7 +197,7 @@ class Face extends Entity implements JsonSerializable {
                $this->markFieldUpdated('descriptor');
        }

-       public function setCreationTime(\DateTime $creationTime): void {
+       public function setCreationTime($creationTime): void {
                if (is_a($creationTime, 'DateTime')) {
                        $this->creationTime = $creationTime;
                } else {
[matias@nube Db]$ 
guystreeter commented 2 years ago

I have created issue https://github.com/nextcloud/server/issues/29466 against Nextcloud server to get OC_image.php fixed.

guystreeter commented 2 years ago

@matiasdelellis Your suggested change worked. I can see the identified people in the Persons tab now.

guystreeter commented 2 years ago

This PR waiting approval will fix OC_Image.php: https://github.com/nextcloud/server/pull/29479

gchmurka123 commented 2 years ago

Thanks, great job, I patched my Nextclouid 22.X version by hand and it finally worked on php 8.x

I hope they add a patch to last stable v22 (they described it as php8 compatible) ;)

jakobroehrl commented 2 years ago

Hi @gchmurka123, how can I install pdlib for PHP8

matiasdelellis commented 2 years ago

Hi everyone, Just comment that thanks to the investigation of @guystreeter in the next version of Nextcloud we will have support for php8 in this application.

Thanks you so much!! 😃 🎉

Kitt3120 commented 2 years ago

Nice! Have been waiting for this since php8 came out. Tried the Face Recognition app occasionally to check if it has been fixed, but eventually just found this GitHub issue and watched it. Looking forward to it, thanks to all people involved :)

guystreeter commented 2 years ago

The necessary fixes have been pulled in the master on github today. https://github.com/nextcloud/server/pull/29479

martadinata666 commented 2 years ago

rebuild my php-fpm to experiment this, but seems another wait for nextcloud release update, is the zip release latest-22.zip, will do?

guystreeter commented 2 years ago

I was mistaken. Only part of the patch-set was pulled in the master branch. It does not include the fixes we need. :(

FernandoMarques-Santos commented 2 years ago

Hi everyone, Just comment that thanks to the investigation of @guystreeter in the next version of Nextcloud we will have support for php8 in this application.

Thanks you so much!! 😃 🎉

Thank you Matias and all! After the nextcloud update, the icing on the cake would be an update to the precompiled PDLib PHP libraries for PHP 8.

BigMichi1 commented 2 years ago

Maybe upvoting https://github.com/oerdnj/deb.sury.org/issues/1663

EWouters commented 2 years ago

The Nextcloud update is now live in version 22.2.1! (https://github.com/nextcloud/server/pull/29519)

martadinata666 commented 2 years ago

Is this compatible after all with nextcloud 22? i got this error on enabling, https://github.com/matiasdelellis/facerecognition/issues/500#issuecomment-913228610

Browse around got #495

/var/www/html $ ./occ app:enable facerecognition
App "Face Recognition" cannot be installed because the following dependencies are not fulfilled: PHP with a version lower than 7.4 is required.
jakobroehrl commented 2 years ago

Can't activate the app with PHP8 an NC 22.2.2. or NC 23 RC2: image

image

EWouters commented 2 years ago

Can't activate the app with PHP8 an NC 22.2.2. or NC 23 RC2

This is off-topic, but you need to follow Hard way.

Kitt3120 commented 2 years ago

Can confirm that it works! Am on arch with Linux Kernel 5.15.2, Nextcloud 22.2.2-1 and Facerecognition v0.7.2, using the aur/dlib-sse-cuda and aur/php-pdlib packages.

matiasdelellis commented 2 years ago

Hi everyone, Please, wait for Nextcloud 22.2.2 (NOT 22.2.1... see https://github.com/nextcloud/server/issues/29678 and https://help.nextcloud.com/t/nc-22-2-1-performance-warning/127169/1), and should test against Facerecognition from git master ...

guystreeter commented 2 years ago

I pulled NC 22.2.2 and built facerecognition from git master, and it seems to be working just fine!

jandr commented 2 years ago

This works fine also in NC 22.2.3 by installing manually facerecognition from git. Hopefully the app from the NC repository will be updated soon to not necessarily require PHP <= 7.4, and will support PHP 8 as well!

matiasdelellis commented 2 years ago

Great. Thank you all very much!. 🎉 😄 Probably tomorrow I will upload an update enabling php 8.. 😬

matiasdelellis commented 2 years ago

Just published on Nextcloud appstrore.. 😄 🎉

Please update from there and report back here.. 😉

FernandoMarques-Santos commented 2 years ago

Ubuntu 20.04, Nextcloud 22.2.3, PHP8.0, I had compiled pdlib to support PHP 8. Today installed the Facerecognition 0.8.5 directly from gitmaster and I confirm that everything is working well. You guys made an excellent job.

However, I got a bit confused by this thread. Are we still needing to compile the pdlib part (see the OP problem)? Or with this facerecognition 0.8.5 (from the app store) we don't need to do that anymore? Sorry I don't see how the update relates to the original problem.

Marginally related to this, are you guys using some of the apps for android? nkming version can show the faces when the facerecognition is pulled directly from the master. It is working pretty well.... butI just wished to not having to compile anything =)

guystreeter commented 2 years ago

I modified by build to pull the latest Nextcloud docker image and only build dlib and pdlib. Then I downloaded and enabled facerecognition from the app store. Everything seems to be working fine.

matiasdelellis commented 2 years ago

Hi @FernandoMarques-Santos

The issue started as a problem that there was no pdlib for php8, but it mutated to the application support for php8 since that was the real problem.

Now Nextcloud added the necessary patches that make this application work with PHP8 and then we enable their support in last release. 😉

About pdlib, except Ubuntu Impish and Fedora 35 no distribution uses php 8 by default. This implies that practically everyone is using third-party repositories to update php, and is simply impossible for me to generate the necessary packages for third party repositories.

So, unfortunately, if you use third-party repositories, you have to compile pdlib on your own.

matiasdelellis commented 2 years ago

Just update package for fedora 35 with php8..

Tomorrow I will probably do the package for Ubuntu Impish.. :wink:

PrivatePuffin commented 2 years ago

So, unfortunately, if you use third-party repositories, you have to compile pdlib on your own.

Calling the official Nextcloud Docker images out for using "third-party repositories" is pretty weird imho