From ed4be105a599c6876356abe91a73e0a15d79712a Mon Sep 17 00:00:00 2001 From: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> Date: Mon, 3 Apr 2023 21:50:40 -0400 Subject: [PATCH 1/6] fix: `dataMimeType` returns the wrong MIME type Signed-off-by: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> --- lib/private/legacy/OC_Image.php | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index 74e82e62b168c..7f45cadbe1d8f 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -123,6 +123,15 @@ public function mimeType(): ?string { return $this->valid() ? $this->mimeType : null; } + /** + * Goofy alias for `mimeType` + * + * @deprecated in favor of `mimeType` + */ + public function dataMimeType(): ?string { + return $this->mimeType(); + } + /** * Returns the width of the image or -1 if no image is loaded. * @@ -352,23 +361,6 @@ public function resource() { return $this->resource; } - /** - * @return string Returns the mimetype of the data. Returns null if the data is not valid. - */ - public function dataMimeType(): ?string { - if (!$this->valid()) { - return null; - } - - switch ($this->mimeType) { - case 'image/png': - case 'image/jpeg': - case 'image/gif': - return $this->mimeType; - default: - return 'image/png'; - } - } /** * @return null|string Returns the raw image data. From 3660abf512e1c9ba2ef54aa615883a37f012e2f9 Mon Sep 17 00:00:00 2001 From: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> Date: Mon, 3 Apr 2023 21:26:04 -0400 Subject: [PATCH 2/6] fix: use preview provider type for thumbnails Fix preview providers generate thumbnails based on source file type instead of their type. This worked because providers were always called with a file of the same type. This is not an assumption that can be made any longer. Note that preview providers don't actually know their type - although they should - so instead make them respect an optional MIME type parameter to avoid potential breaking changes. Signed-off-by: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> --- lib/private/Preview/Generator.php | 14 ++++++++++++-- lib/private/Preview/GeneratorHelper.php | 4 ++-- lib/private/Preview/Image.php | 3 ++- lib/private/legacy/OC_Image.php | 4 ++++ 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 47f2952c6e6b8..038e7d51946fe 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -271,7 +271,7 @@ private function getSmallImagePreview(ISimpleFolder $previewFolder, File $file, continue; } - $preview = $this->helper->getThumbnail($provider, $file, 256, 256, $crop); + $preview = $this->helper->getThumbnail($provider, $file, 256, 256, $crop, $mimeType); if (!($preview instanceof IImage)) { continue; @@ -437,7 +437,17 @@ private function getMaxPreview(ISimpleFolder $previewFolder, File $file, $mimeTy $previewConcurrency = $this->getNumConcurrentPreviews('preview_concurrency_new'); $sem = self::guardWithSemaphore(self::SEMAPHORE_ID_NEW, $previewConcurrency); try { - $preview = $this->helper->getThumbnail($provider, $file, $maxWidth, $maxHeight); + // Although we know the provider here, and it *should* know + // its own MIME type, note that it actually doesn't - so we + // have to pass it. + // + // *`IProviderV2->getMimeType`* + // + // > Regex with the mimetypes that are supported by this + // provider + // + // This is also the case for `$this->getSmallImagePreview`. + $preview = $this->helper->getThumbnail($provider, $file, $maxWidth, $maxHeight, false, $mimeType); } finally { self::unguardWithSemaphore($sem); } diff --git a/lib/private/Preview/GeneratorHelper.php b/lib/private/Preview/GeneratorHelper.php index 64714a9f899ca..16ef7ff64b709 100644 --- a/lib/private/Preview/GeneratorHelper.php +++ b/lib/private/Preview/GeneratorHelper.php @@ -57,11 +57,11 @@ public function __construct(IRootFolder $rootFolder, IConfig $config) { * * @return bool|IImage */ - public function getThumbnail(IProviderV2 $provider, File $file, $maxWidth, $maxHeight, bool $crop = false) { + public function getThumbnail(IProviderV2 $provider, File $file, $maxWidth, $maxHeight, bool $crop = false, string $mimeType = null) { if ($provider instanceof Imaginary) { return $provider->getCroppedThumbnail($file, $maxWidth, $maxHeight, $crop) ?? false; } - return $provider->getThumbnail($file, $maxWidth, $maxHeight) ?? false; + return $provider->getThumbnail($file, $maxWidth, $maxHeight, $mimeType) ?? false; } /** diff --git a/lib/private/Preview/Image.php b/lib/private/Preview/Image.php index 95b66a922fd14..cfa5ca2ed3b9d 100644 --- a/lib/private/Preview/Image.php +++ b/lib/private/Preview/Image.php @@ -36,7 +36,7 @@ abstract class Image extends ProviderV2 { /** * {@inheritDoc} */ - public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { + public function getThumbnail(File $file, int $maxX, int $maxY, string $mimeType = null): ?IImage { $maxSizeForImages = \OC::$server->getConfig()->getSystemValueInt('preview_max_filesize_image', 50); $size = $file->getSize(); @@ -55,6 +55,7 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage { if ($image->valid()) { $image->scaleDownToFit($maxX, $maxY); + if ($mimeType) $image->setMimeType($mimeType); return $image; } diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index 7f45cadbe1d8f..b63fdbcd991af 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -123,6 +123,10 @@ public function mimeType(): ?string { return $this->valid() ? $this->mimeType : null; } + public function setMimeType(string $mimeType): void { + $this->mimeType = $mimeType; + } + /** * Goofy alias for `mimeType` * From 5a87d4a97a1526e563d3eea1e253b422a2eb9a1e Mon Sep 17 00:00:00 2001 From: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> Date: Mon, 3 Apr 2023 22:02:27 -0400 Subject: [PATCH 3/6] refactor: add option to check preview availability for a specific format Signed-off-by: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> --- lib/private/PreviewManager.php | 8 +++++--- lib/public/IPreview.php | 3 ++- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/private/PreviewManager.php b/lib/private/PreviewManager.php index dd6b6ba8ee112..b21f79b146fba 100644 --- a/lib/private/PreviewManager.php +++ b/lib/private/PreviewManager.php @@ -243,13 +243,15 @@ public function isMimeSupported($mimeType = '*') { /** * Check if a preview can be generated for a file */ - public function isAvailable(\OCP\Files\FileInfo $file): bool { + public function isAvailable(\OCP\Files\FileInfo $file, ?string $previewMimeType = null): bool { + if (!$previewMimeType) $previewMimeType = $file->getMimeType(); + if (!$this->config->getSystemValue('enable_previews', true)) { return false; } $this->registerCoreProviders(); - if (!$this->isMimeSupported($file->getMimetype())) { + if (!$this->isMimeSupported($previewMimeType)) { return false; } @@ -259,7 +261,7 @@ public function isAvailable(\OCP\Files\FileInfo $file): bool { } foreach ($this->providers as $supportedMimeType => $providers) { - if (preg_match($supportedMimeType, $file->getMimetype())) { + if (preg_match($supportedMimeType, $previewMimeType)) { foreach ($providers as $providerClosure) { $provider = $this->helper->getProvider($providerClosure); if (!($provider instanceof IProviderV2)) { diff --git a/lib/public/IPreview.php b/lib/public/IPreview.php index 8483587d60e33..ed468dbee7b19 100644 --- a/lib/public/IPreview.php +++ b/lib/public/IPreview.php @@ -108,10 +108,11 @@ public function isMimeSupported($mimeType = '*'); * Check if a preview can be generated for a file * * @param \OCP\Files\FileInfo $file + * @param ?string $previewMimeType MIME type of the preview to be generated * @return bool * @since 8.0.0 */ - public function isAvailable(\OCP\Files\FileInfo $file); + public function isAvailable(\OCP\Files\FileInfo $file, string $previewMimeType); /** * Generates previews of a file From ba3499bffff7bd812b9d25d630f9d2c8b05bab00 Mon Sep 17 00:00:00 2001 From: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> Date: Mon, 3 Apr 2023 21:46:09 -0400 Subject: [PATCH 4/6] feat: add `preview format` setting Allow preview format to be specified to be a specific type for all files. Previously, preview format was the same type as the original file. Format can be jpeg, png, gif, webp, or avif. Signed-off-by: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> --- lib/private/Preview/Generator.php | 62 ++++++++++++++++++++++++++++++- 1 file changed, 60 insertions(+), 2 deletions(-) diff --git a/lib/private/Preview/Generator.php b/lib/private/Preview/Generator.php index 038e7d51946fe..93858a092db6b 100644 --- a/lib/private/Preview/Generator.php +++ b/lib/private/Preview/Generator.php @@ -51,6 +51,8 @@ class Generator { public const SEMAPHORE_ID_ALL = 0x0a11; public const SEMAPHORE_ID_NEW = 0x07ea; + private \OCP\ILogger $logger; + /** @var IPreview */ private $previewManager; /** @var IConfig */ @@ -64,6 +66,14 @@ class Generator { /** @var IEventDispatcher */ private $eventDispatcher; + /** + * GD image type for the preview format setting + * + * `null` if the preview format setting is not set (=> use default handling + * instead). + */ + private null|int $previewImageType; + public function __construct( IConfig $config, IPreview $previewManager, @@ -72,12 +82,49 @@ public function __construct( EventDispatcherInterface $legacyEventDispatcher, IEventDispatcher $eventDispatcher ) { + $this->logger = \OC::$server->getLogger(); + $this->config = $config; $this->previewManager = $previewManager; $this->appData = $appData; $this->helper = $helper; $this->legacyEventDispatcher = $legacyEventDispatcher; $this->eventDispatcher = $eventDispatcher; + + $this->setPreviewImageType(); + } + + private function setPreviewImageType(): void { + // Only allow previews to be common image formats that there is a clear + // need for. + // See https://developer.mozilla.org/en-US/docs/Web/HTTP/Basics_of_HTTP/MIME_types#image_types + $ALLOWED_FORMATS = ['AVIF', 'GIF', 'JPEG', 'PNG', 'WEBP']; + $FALLBACK_MESSAGE = 'Falling back to the default preview format.'; + + $this->previewImageType = null; + + $previewFormat = strtoupper( + $this->config->getAppValue('preview', 'format') + ); + + if (!$previewFormat) return; + if (!in_array($previewFormat, $ALLOWED_FORMATS)) { + $this->logger->error( + 'Preview format must be one of [' . + implode(', ', $ALLOWED_FORMATS) . + "]; got $previewFormat. $FALLBACK_MESSAGE" + ); + return; + } + if (!(imagetypes() & constant("IMG_$previewFormat"))) { + $this->logger->error( + 'The installation does not support preview format ' . + "$previewFormat. $FALLBACK_MESSAGE" + ); + return; + } + + $this->previewImageType = constant("IMAGETYPE_$previewFormat"); } /** @@ -132,8 +179,14 @@ public function generatePreviews(File $file, array $specifications, $mimeType = throw new NotFoundException('Cannot read file'); } - if ($mimeType === null) { - $mimeType = $file->getMimeType(); + // `$mimeType` argument overrides preview format setting overrides + // default to the same MIME type as the file it is a preview for + if (!$mimeType) { + if (!is_null($this->previewImageType)) { + $mimeType = image_type_to_mime_type($this->previewImageType); + } else { + $mimeType = $file->getMimeType(); + } } $previewFolder = $this->getPreviewFolder($file); @@ -702,6 +755,9 @@ private function getPreviewFolder(File $file) { * @param string $mimeType * @return null|string * @throws \InvalidArgumentException + * + * @deprecated this is goofy + * @see https://www.php.net/manual/en/function.image-type-to-extension.php */ private function getExtention($mimeType) { switch ($mimeType) { @@ -711,6 +767,8 @@ private function getExtention($mimeType) { return 'jpg'; case 'image/gif': return 'gif'; + case 'image/webp': + return 'webp'; default: throw new \InvalidArgumentException('Not a valid mimetype: "' . $mimeType . '"'); } From d38f33f076c3c8bdcb364e6b2ae5209125ef073b Mon Sep 17 00:00:00 2001 From: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> Date: Mon, 3 Apr 2023 21:58:39 -0400 Subject: [PATCH 5/6] feat: add `preview quality` setting for lossy compression Signed-off-by: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> --- lib/private/legacy/OC_Image.php | 45 ++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index b63fdbcd991af..8deddcbba1bbf 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -52,7 +52,12 @@ class OC_Image implements \OCP\IImage { // Default memory limit for images to load (256 MBytes). protected const DEFAULT_MEMORY_LIMIT = 256; - // Default quality for jpeg images + /** + * Default quality for jpeg images + * + * @deprecated this choice should be left to the image library + * @see https://www.php.net/manual/en/function.imagejpeg.php#refsect1-function.imagejpeg-parameters + */ protected const DEFAULT_JPEG_QUALITY = 80; /** @var false|resource|\GdImage */ @@ -373,6 +378,9 @@ public function data(): ?string { if (!$this->valid()) { return null; } + + $quality = $this->getQuality(); + ob_start(); switch ($this->mimeType) { case "image/png": @@ -381,12 +389,14 @@ public function data(): ?string { case "image/jpeg": /** @psalm-suppress InvalidScalarArgument */ imageinterlace($this->resource, (PHP_VERSION_ID >= 80000 ? true : 1)); - $quality = $this->getJpegQuality(); - $res = imagejpeg($this->resource, null, $quality); + $res = imagejpeg($this->resource, null, $this->getJpegQuality()); break; case "image/gif": $res = imagegif($this->resource); break; + case "image/webp": + $res = imagewebp($this->resource, null, $quality); + break; default: $res = imagepng($this->resource); $this->logger->info('OC_Image->data. Could not guess mime-type, defaulting to png', ['app' => 'core']); @@ -398,6 +408,33 @@ public function data(): ?string { return ob_get_clean(); } + /** + * Gets the image quality with which to write lossy image previews + * + * @return int the image quality on a scale of 0-100, or -1 for the default + */ + private function getQuality(): int { + $quality = $this->config->getAppValue('preview', 'quality'); + + if ($quality && (!intval($quality) || $quality < 0 || $quality > 100)) { + $this->logger->error( + 'Preview quality must be an integer from 0 to 100; got ' . + "$quality. Falling back to the default preview quality." + ); + } + + // If quality is not set by the user or is invalid, then set it to the + // GD default value to entrust choosing a well-optimized default + // quality to the image library (which should make a much more educated + // choice than we would make). Hopefully GD doesn't mangle this idea... + // but it appears they do - WebP default should be 75 yet is 80. + // + // See https://developers.google.com/speed/webp/docs/cwebp#options + if (!$quality) $quality = -1; + + return (int) $quality; + } + /** * @return string - base64 encoded, which is suitable for embedding in a VCard. */ @@ -407,6 +444,8 @@ public function __toString() { /** * @return int + * + * @deprecated */ protected function getJpegQuality(): int { $quality = $this->config->getAppValue('preview', 'jpeg_quality', (string) self::DEFAULT_JPEG_QUALITY); From 8d09ec98d5e0de55ed1aad10152a118096ef792f Mon Sep 17 00:00:00 2001 From: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> Date: Mon, 3 Apr 2023 21:59:42 -0400 Subject: [PATCH 6/6] feat: add AVIF support Signed-off-by: Anderson Entwistle <46688047+aentwist@users.noreply.github.com> --- lib/composer/composer/autoload_classmap.php | 1 + lib/composer/composer/autoload_static.php | 1 + lib/private/Preview/Avif.php | 19 +++++++++++++++++++ lib/private/Preview/Generator.php | 2 ++ lib/private/PreviewManager.php | 2 ++ lib/private/legacy/OC_Image.php | 12 ++++++++++++ resources/config/mimetypemapping.dist.json | 1 + 7 files changed, 38 insertions(+) create mode 100644 lib/private/Preview/Avif.php diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 40d0c4f0cd1b2..fd3ee6ed7bed1 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1394,6 +1394,7 @@ 'OC\\OCS\\Result' => $baseDir . '/lib/private/OCS/Result.php', 'OC\\PreviewManager' => $baseDir . '/lib/private/PreviewManager.php', 'OC\\PreviewNotAvailableException' => $baseDir . '/lib/private/PreviewNotAvailableException.php', + 'OC\\Preview\\Avif' => $baseDir . '/lib/private/Preview/Avif.php', 'OC\\Preview\\BMP' => $baseDir . '/lib/private/Preview/BMP.php', 'OC\\Preview\\BackgroundCleanupJob' => $baseDir . '/lib/private/Preview/BackgroundCleanupJob.php', 'OC\\Preview\\Bitmap' => $baseDir . '/lib/private/Preview/Bitmap.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 65021f54adbba..7152b8f0f4a5b 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1427,6 +1427,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\OCS\\Result' => __DIR__ . '/../../..' . '/lib/private/OCS/Result.php', 'OC\\PreviewManager' => __DIR__ . '/../../..' . '/lib/private/PreviewManager.php', 'OC\\PreviewNotAvailableException' => __DIR__ . '/../../..' . '/lib/private/PreviewNotAvailableException.php', + 'OC\\Preview\\Avif' => __DIR__ . '/../../..' . '/lib/private/Preview/Avif.php', 'OC\\Preview\\BMP' => __DIR__ . '/../../..' . '/lib/private/Preview/BMP.php', 'OC\\Preview\\BackgroundCleanupJob' => __DIR__ . '/../../..' . '/lib/private/Preview/BackgroundCleanupJob.php', 'OC\\Preview\\Bitmap' => __DIR__ . '/../../..' . '/lib/private/Preview/Bitmap.php', diff --git a/lib/private/Preview/Avif.php b/lib/private/Preview/Avif.php new file mode 100644 index 0000000000000..4fdfc7698c855 --- /dev/null +++ b/lib/private/Preview/Avif.php @@ -0,0 +1,19 @@ +defaultProviders = $this->config->getSystemValue('enabledPreviewProviders', array_merge([ @@ -368,6 +369,7 @@ protected function registerCoreProviders() { $this->registerCoreProvider(Preview\BMP::class, '/image\/bmp/'); $this->registerCoreProvider(Preview\XBitmap::class, '/image\/x-xbitmap/'); $this->registerCoreProvider(Preview\WebP::class, '/image\/webp/'); + $this->registerCoreProvider(Preview\Avif::class, '/image\/avif/'); $this->registerCoreProvider(Preview\Krita::class, '/application\/x-krita/'); $this->registerCoreProvider(Preview\MP3::class, '/audio\/mpeg/'); $this->registerCoreProvider(Preview\OpenDocument::class, '/application\/vnd.oasis.opendocument.*/'); diff --git a/lib/private/legacy/OC_Image.php b/lib/private/legacy/OC_Image.php index 8deddcbba1bbf..06e60bcb2bf0b 100644 --- a/lib/private/legacy/OC_Image.php +++ b/lib/private/legacy/OC_Image.php @@ -397,6 +397,9 @@ public function data(): ?string { case "image/webp": $res = imagewebp($this->resource, null, $quality); break; + case "image/avif": + $res = imageavif($this->resource, null, $quality); + break; default: $res = imagepng($this->resource); $this->logger->info('OC_Image->data. Could not guess mime-type, defaulting to png', ['app' => 'core']); @@ -762,6 +765,15 @@ public function loadFromFile($imagePath = false) { $this->logger->debug('OC_Image->loadFromFile, webp images not supported: ' . $imagePath, ['app' => 'core']); } break; + case IMAGETYPE_AVIF: + if (imagetypes() & IMG_AVIF) { + if (!$this->checkImageSize($imagePath)) return false; + + $this->resource = imagecreatefromavif($imagePath); + } else { + $this->logger->debug("OC_Image->loadFromFile: installation does not support AVIF; got $imagePath", ['app' => 'core']); + } + break; /* case IMAGETYPE_TIFF_II: // (intel byte order) break; diff --git a/resources/config/mimetypemapping.dist.json b/resources/config/mimetypemapping.dist.json index 7b631466972fd..4ce98da05a734 100644 --- a/resources/config/mimetypemapping.dist.json +++ b/resources/config/mimetypemapping.dist.json @@ -16,6 +16,7 @@ "arw": ["image/x-dcraw"], "asciidoc": ["text/asciidoc", "text/plain"], "avi": ["video/x-msvideo"], + "avif": ["image/avif"], "bash": ["text/x-shellscript"], "bat": ["application/x-msdos-program"], "bin": ["application/x-bin"],