Fix preview generation for mp3 files

[prsnbrg]

Description

Related Issue

• Fixes uploading .mp3 file shows weird logs #34900

Motivation and Context

How Has This Been Tested?

• test environment: oC 10.6, oC 10.8 with encryption
• test case 1: Load file list with mp3 file before and after change- compare log output (level 3)
• test case 2:
• ...

Screenshots (if appropriate):

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Database schema changes (next release will require increase of minor version instead of patch)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests only (no source changes)

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:
• Changelog item, see TEMPLATE

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[prsnbrg]

After this fix there is still an error with imagecreatefromstring. The string given to the method is /var/www/owncloud/core/img/filetypes/audio.svg.
The following message appears: imagecreatefromstring(): Data is not in a recognized format at /lib/private/legacy/image.php#L573

[JammingBen]

Although this is legacy code, I'm not too confident about removing \OC\Files\Filesystem::file_get_contents(). As the comment above states, we might need this to properly handle encrypted files. Also this method is not suited for handling SVG files, hence the error with imagecreatefromstring().

IMO the best approach would be to adjust getNoCoverThumbnail() (see https://github.com/owncloud/core/blob/master/lib/private/Preview/MP3.php#L83). Some logic to convert SVGs should do the trick here (plus it only affects mp3-files without a cover). Something like:

$stream = \fopen($icon, 'r');
$content = \stream_get_contents($stream);
\fclose($stream);

$svg = new \Imagick();
$svg->setBackgroundColor(new \ImagickPixel('transparent'));
$svg->setResolution(1200, 1200);
$svg->readImageBlob($content);
$svg->setImageFormat('png32');

$image = new \OC_Image();
$image->loadFromData($svg);
return $image->valid() ? $image : false;

[prsnbrg]

Thank you very much.

I tried a few things with your code example and unfortunately run into an error.
While the page is loading, the icon is displayed perfectly and after loading it is warped.

While loading

After loading is finished

@JammingBen Do you have an Idea about this?

[JammingBen]

What you see directly after uploading is the plain SVG image, therefore it looks super sharp. Shortly after, the preview manager detects that this is a mp3 file which could have a thumbnail. This is where your code comes into play and renders a "no cover thumbnail" based on the SVG file.

However, for me the icon doesn't look pixelated. Do you use $svg->setResolution(1200, 1200)? What happens if you increase those values?

[prsnbrg]

Yes this is the result with your code.

The icons from my last post was on a hosted server, today I tested this on my own Linux Server and got this icon:

What can be the reason for the different behavior?
My specifications from today:

• OS: Raspbian GNU/Linux 10 (buster)
• Apache2-Version: Apache/2.4.38 (Raspbian)
• PHP-Version: 7.3.29-1~deb10u1
• MySQL/MariaDB-Version: 15.1 Distrib 10.3.29-MariaDB
• oC Versions: 10.8.0.4 and 10.7.0.4

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

[ownclouders]

💥 Acceptance tests pipeline apiWebdavMove1-mariadb10.2-php7.4 failed. The build has been cancelled.

https://drone.owncloud.com/owncloud/core/32000/98/1

[JammingBen]

Okay that is weird. Differences in PHP and/or imagick versions on both of these servers?

Note that this is also not an optimal solution. The best approach would be to ignore the thumbnail creation for mp3s without cover and use the svg image instead. Unfortunately, the current architecture does not allow that easily. It just divides into "needs thumbnail" and "doesn't need thumbnail". There is no category for "may need a thumbnail based in meta data".

[prsnbrg]

Is there a way to prevent overwriting the icon when there is no preview.

Even if getThumbnail returns false, the icon is changed for me.