search

JoomGalleryfriends / JG4-dev

Development repository for JoomGallery v4.x
GNU General Public License v3.0
10 stars 6 forks source link

Overhaul of Multiple upload PR #103

Closed Elfangor93 closed 1 year ago

Elfangor93 commented 1 year ago

This is a overhaul of the PR #94.

In principle this PR adds exactly the same functionality as PR #94 so please take a look at the description there. The main difference is that code-wise the functionality is approached in a much cleaner and more sustainable way. Insead of hacking the functionality into uppy, it extends uppy by the help of an uppy-plugin called "jgProcessor" and a template override of the uppy dashboard plugin called "jgDashboard".

This way the issue #100 was solved.

AlexanderSupp commented 1 year ago

I uninstalled JG4 and installed uppy-plugins only. An error occurred by edit Global-Configuration. Screenshot 2023-06-05 180318

I ignored this warning and continue the test.

AlexanderSupp commented 1 year ago

I've unapproved an image and a similar warning comes up. Screenshot 2023-06-05 182255 I continue to test.

Elfangor93 commented 1 year ago

@AlexanderSupp Can you please share your environment info (PHP, DB version)?

AlexanderSupp commented 1 year ago

My environment as txt-data. systeminfo-2023-06-05T19 23 20+00 00.txt

AlexanderSupp commented 1 year ago

Some data from my tests. 184 images selected, with 1.1 GB total size Drive SSD, utilization approx. 5% Main memory usage 26 GB of 32 GB CPU usage 20-30% at 4.5-4.7 GHz Running time about 7 minutes There still seems room for improvement. It seems the system uses only 1 CPU core. It should run faster with 12 cores. But everything runs well. Further tests with 158, 118 and 92 images each went through in one go without any problems. Number of concurrent processes must remain at 1. At 3 there are errors. Entry of title, description, author for individual images not yet tested.

Anomalies: There are sometimes gaps in the preview of the images. See example below. However, all images are displayed and processed. Looks very good overall. Screenshot 2023-06-05 232921

AlexanderSupp commented 1 year ago

I started a test with the environment of my provider and everything runs without warnings. However, I aborted the test with my 184 images because the upload would have taken more than 2 hours. After 15 images the result was OK.

My environment: PHP Built On Linux sh11020 3.10.0-1160.88.1.el7.x86_64 #1 SMP Tue Mar 7 15:41:52 UTC 2023 x86_64 Database Type mysql Database Version 5.7.41-log Database Collation utf8mb4_unicode_ci Database Connection Collation utf8mb4_general_ci Database Connection Encryption None Database Server Supports Connection Encryption Yes PHP Version 8.1.18 Web Server Apache WebServer to PHP Interface fpm-fcgi Joomla! Version Joomla! 4.3.2 Stable [ Bora ] 30-May-2023 16:00 GMT User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/113.0 Setting Value PHP Built On Linux sh11020 3.10.0-1160.88.1.el7.x86_64 #1 SMP Tue Mar 7 15:41:52 UTC 2023 x86_64 Database Type mysql Database Version 5.7.41-log Database Collation utf8mb4_unicode_ci Database Connection Collation utf8mb4_general_ci Database Connection Encryption None Database Server Supports Connection Encryption Yes PHP Version 8.1.18 Web Server Apache WebServer to PHP Interface fpm-fcgi Joomla! Version Joomla! 4.3.2 Stable [ Bora ] 30-May-2023 16:00 GMT User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/113.0 Setting Value PHP Built On Linux sh11020 3.10.0-1160.88.1.el7.x86_64 #1 SMP Tue Mar 7 15:41:52 UTC 2023 x86_64 Database Type mysql Database Version 5.7.41-log Database Collation utf8mb4_unicode_ci Database Connection Collation utf8mb4_general_ci Database Connection Encryption None Database Server Supports Connection Encryption Yes PHP Version 8.1.18 Web Server Apache WebServer to PHP Interface fpm-fcgi Joomla! Version Joomla! 4.3.2 Stable [ Bora ] 30-May-2023 16:00 GMT User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:109.0) Gecko/20100101 Firefox/113.0

Elfangor93 commented 1 year ago

I've unapproved an image and a similar warning comes up.

This warning has nothing to do with this PR will be fixed later on. Can you please create a separate issue for it?

Elfangor93 commented 1 year ago

Anomalies: There are sometimes gaps in the preview of the images. See example below. However, all images are displayed and processed.

This is nothing I can fix within JoomGallery. But you can open an issue on that in the uppy repository.

MrMusic commented 1 year ago

I did some upload tests. The upload works quite well, even with a large number of images. 👍 Nevertheless, I noticed the following points:

  1. The problem with uploading very large images with image processor GD and low memory limit still exists: https://github.com/JoomGalleryfriends/JG4-dev/pull/94#issuecomment-1554279992

  2. When 'Debug mode' is activated The same debug information is displayed for all images. Probably from the first image.

  3. Error that in the upload form indvidually entered metadata are only partially taken over. There is a new fix for this problem. After adopting this fix https://github.com/transloadit/uppy/pull/4490/files into the file uppy-uploader.js all entered data will be taken over correctly. Perhaps we can already apply this fix even if no new release has been published yet?

General notes:

  1. Priority There are three possibilities for filling the image database fields title, description and author:
    • general title, general description, general author
    • individual title, description, author for each image
    • Replace with the metadata

If set, the replace with metadata have the highest priority. This is followed by the data assigned individually for each image. The general data have the lowest priority. From my point of view, the priority order is correct.

  1. Alias When uploading and the title is taken from the metadata, the alias is not changed accordingly. Should this be done?

  2. Configuration set: The 'Nmb parallel processes' in the Uploads tab could be marked as 'sensitive'. After all, this can easily damage the upload process.

  3. The 'Maximum Size (in MB)' in the common settings tab Currently, clicking on the 'Edit' button takes you to the Media Options. However, the current JoomGallery configuration set is then 'checked out'. I would suggest to removing the Edit button and improving the description a bit.

MrMusic commented 1 year ago

Anomalies: There are sometimes gaps in the preview of the images. See example below. However, all images are displayed and processed.

Yes, I can confirm the problem. The cause is probably long file names that lead to a line break. That is why this shift occurs in the following line.

To get around the problem, you could increase the available space for the file names. The following worked for me, but it is a 'bad' hack. Add the following in media/com_joomgallery/css/admin.css:

.uppy-dashboard-item-name {
  min-height: 35px;
}

Maybe someone has a better suggestion? Maybe more 'shorten' the text?

AlexanderSupp commented 1 year ago

@MrMusic Thanks, great to know. From my view this pr is ready to merge into main.

Elfangor93 commented 1 year ago

When uploading and the title is taken from the metadata, the alias is not changed accordingly. Should this be done?

@MrMusic Please open an new issue for that.

Elfangor93 commented 1 year ago

Perhaps we can already apply this fix even if no new release has been published yet?

No apparently not. We have to wait for a new release.

Elfangor93 commented 1 year ago

When 'Debug mode' is activated The same debug information is displayed for all images. Probably from the first image.

This is a very tricky one. I will fix it in a separate PR. Please open a new issue for that.

AlexanderSupp commented 1 year ago

I tested once more in my previous defined environment with the following results. GD Library, 118 Image files, 817 MB total: Parallel processes 1 = OK Parallel processes 8 = OK Parallel processes 30 = OK

ImageMagick, 118 Image files, 817 MB total: Parallel processes 1 = OK Parallel processes 3 = failed Parallel processes 30 = failed Debug info here: Error-Multiple-Upload-ImageMagick-Parallel-Processes-3.txt

Elfangor93 commented 1 year ago

The problem with uploading very large images with image processor GD and low memory limit still exists

@MrMusic Fixing this problem is not possible, but now PHP Fatal error are catched from the ajax request and displayed in the debug information. Ths way the user sees what is the problem.

Elfangor93 commented 1 year ago

@AlexanderSupp You were right. When using ImageMagick multiple temporary files are created to store intermediate processing steps. This temporary files had hard coded names like 'tmp_img.jpg' and were conflicting when multiple processes were run in parallel on the server. --> Should be fixed now!

AlexanderSupp commented 1 year ago

@Elfangor93 Great, ImageMagick works also. 158 files, 969 MB total Parallel processes 1 = about 6 minutes Parallel processes 8 = about 2 minutes Parallel processes 30 = about 2 minutes So the best for my machine is 8. Improvement by a factor of 3. Thanks a lot.

AlexanderSupp commented 1 year ago

If set, the replace with metadata have the highest priority. This is followed by the data assigned individually for each image. The general data have the lowest priority. From my point of view, the priority order is correct.

That's very fine.

MrMusic commented 1 year ago

I can confirm @AlexanderSupp tests: up to max. 8 processes make sense. An even higher number no longer brings any improvement. After, as far as possible, the problems have been solved, i would also be in favour of merging this PR.

Elfangor93 commented 1 year ago

Thank you all for testing! This PR is a big milestone towards the first stable release...