From 74bf9806b0fd17c82a5d3048d03a8dc735a2ce04 Mon Sep 17 00:00:00 2001 From: Olivier Paroz Date: Tue, 7 Apr 2015 16:45:16 +0200 Subject: [PATCH] Introducing the maximum size preview The first time we're asked to generate a preview we'll generate one of the maximum dimension indicated in the configuration and all future resizing requests will be done on that preview in order to not waste time converting the same file over and over. One of the fixes required for #12465 --- config/config.sample.php | 4 +- lib/private/preview.php | 365 ++++++++++++++++++++++++++------------- tests/lib/preview.php | 90 ++++++++-- 3 files changed, 320 insertions(+), 139 deletions(-) diff --git a/config/config.sample.php b/config/config.sample.php index 60932ab7d9..29f7c2ad55 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -620,12 +620,12 @@ $CONFIG = array( * The maximum width, in pixels, of a preview. A value of ``null`` means there * is no limit. */ -'preview_max_x' => null, +'preview_max_x' => 2048, /** * The maximum height, in pixels, of a preview. A value of ``null`` means there * is no limit. */ -'preview_max_y' => null, +'preview_max_y' => 2048, /** * If a lot of small pictures are stored on the ownCloud instance and the * preview system generates blurry previews, you might want to consider setting diff --git a/lib/private/preview.php b/lib/private/preview.php index f8e90cafbc..eab60e1086 100644 --- a/lib/private/preview.php +++ b/lib/private/preview.php @@ -3,11 +3,11 @@ * @author Björn Schießle * @author Frank Karlitschek * @author Georg Ehrke - * @author Georg Ehrke * @author Joas Schilling * @author Jörn Friedrich Dreyer * @author Lukas Reschke * @author Morris Jobke + * @author Olivier Paroz * @author Robin Appelman * @author Robin McCorkell * @author Thomas Müller @@ -95,9 +95,9 @@ class Preview { $this->userView = new \OC\Files\View('/' . $user); //set config - $this->configMaxX = \OC_Config::getValue('preview_max_x', null); - $this->configMaxY = \OC_Config::getValue('preview_max_y', null); - $this->maxScaleFactor = \OC_Config::getValue('preview_max_scale_factor', 2); + $this->configMaxX = \OC::$server->getConfig()->getSystemValue('preview_max_x', 2048); + $this->configMaxY = \OC::$server->getConfig()->getSystemValue('preview_max_y', 2048); + $this->maxScaleFactor = \OC::$server->getConfig()->getSystemValue('preview_max_scale_factor', 2); //save parameters $this->setFile($file); @@ -246,7 +246,7 @@ class Preview { $configMaxX = $this->getConfigMaxX(); if (!is_null($configMaxX)) { if ($maxX > $configMaxX) { - \OC_Log::write('core', 'maxX reduced from ' . $maxX . ' to ' . $configMaxX, \OC_Log::DEBUG); + \OCP\Util::writeLog('core', 'maxX reduced from ' . $maxX . ' to ' . $configMaxX, \OCP\Util::DEBUG); $maxX = $configMaxX; } } @@ -267,7 +267,7 @@ class Preview { $configMaxY = $this->getConfigMaxY(); if (!is_null($configMaxY)) { if ($maxY > $configMaxY) { - \OC_Log::write('core', 'maxX reduced from ' . $maxY . ' to ' . $configMaxY, \OC_Log::DEBUG); + \OCP\Util::writeLog('core', 'maxX reduced from ' . $maxY . ' to ' . $configMaxY, \OCP\Util::DEBUG); $maxY = $configMaxY; } } @@ -304,12 +304,12 @@ class Preview { public function isFileValid() { $file = $this->getFile(); if ($file === '') { - \OC_Log::write('core', 'No filename passed', \OC_Log::DEBUG); + \OCP\Util::writeLog('core', 'No filename passed', \OCP\Util::DEBUG); return false; } if (!$this->fileView->file_exists($file)) { - \OC_Log::write('core', 'File:"' . $file . '" not found', \OC_Log::DEBUG); + \OCP\Util::writeLog('core', 'File:"' . $file . '" not found', \OCP\Util::DEBUG); return false; } @@ -321,9 +321,7 @@ class Preview { * @return bool */ public function deletePreview() { - $file = $this->getFile(); - - $fileInfo = $this->getFileInfo($file); + $fileInfo = $this->getFileInfo(); if($fileInfo !== null && $fileInfo !== false) { $fileId = $fileInfo->getId(); @@ -357,7 +355,8 @@ class Preview { } /** - * check if thumbnail or bigger version of thumbnail of file is cached + * Checks if thumbnail or bigger version of thumbnail of file is already cached + * * @param int $fileId fileId of the original image * @return string|false path to thumbnail if it exists or false */ @@ -366,9 +365,11 @@ class Preview { return false; } + // This gives us a calculated path to a preview of asked dimensions + // thumbnailFolder/fileId/my_image--.png $preview = $this->buildCachePath($fileId); - //does a preview with the wanted height and width already exist? + // This checks if a preview exists at that location if ($this->userView->file_exists($preview)) { return $preview; } @@ -377,34 +378,39 @@ class Preview { } /** - * check if a bigger version of thumbnail of file is cached + * Checks if a bigger version of a file preview is cached and if not + * return the preview of max allowed dimensions + * * @param int $fileId fileId of the original image + * * @return string|false path to bigger thumbnail if it exists or false - */ + */ private function isCachedBigger($fileId) { if (is_null($fileId)) { return false; } - // in order to not loose quality we better generate aspect preserving previews from the original file - if ($this->keepAspect) { - return false; - } - $maxX = $this->getMaxX(); //array for usable cached thumbnails + // FIXME: Checking only the width could lead to issues $possibleThumbnails = $this->getPossibleThumbnails($fileId); foreach ($possibleThumbnails as $width => $path) { - if ($width < $maxX) { + if ($width === 'max' || $width < $maxX) { continue; } else { return $path; } } + // At this stage, we didn't find a preview, so if the folder is not empty, + // we return the max preview we generated on the first run + if ($possibleThumbnails) { + return $possibleThumbnails['max']; + } + return false; } @@ -421,7 +427,7 @@ class Preview { $previewPath = $this->getPreviewPath($fileId); - $wantedAspectRatio = (float) ($this->getMaxX() / $this->getMaxY()); + $wantedAspectRatio = (float)($this->getMaxX() / $this->getMaxY()); //array for usable cached thumbnails $possibleThumbnails = array(); @@ -429,6 +435,11 @@ class Preview { $allThumbnails = $this->userView->getDirectoryContent($previewPath); foreach ($allThumbnails as $thumbnail) { $name = rtrim($thumbnail['name'], '.png'); + // Always add the max preview to the array + if (strpos($name, 'max')) { + $possibleThumbnails['max'] = $thumbnail['path']; + continue; + } list($x, $y, $aspectRatio) = $this->getDimensionsFromFilename($name); if (abs($aspectRatio - $wantedAspectRatio) >= 0.000001 @@ -482,7 +493,11 @@ class Preview { } /** - * return a preview of a file + * Returns a preview of a file + * + * The cache is searched first and if nothing usable was found then a preview is + * generated by one of the providers + * * @return \OCP\IImage */ public function getPreview() { @@ -491,76 +506,22 @@ class Preview { } $this->preview = null; - $file = $this->getFile(); - $maxX = $this->getMaxX(); - $maxY = $this->getMaxY(); - $scalingUp = $this->getScalingUp(); - - $fileInfo = $this->getFileInfo($file); - if($fileInfo === null || $fileInfo === false) { + $fileInfo = $this->getFileInfo(); + if ($fileInfo === null || $fileInfo === false) { return new \OC_Image(); } - $fileId = $fileInfo->getId(); + $fileId = $fileInfo->getId(); $cached = $this->isCached($fileId); if ($cached) { - $stream = $this->userView->fopen($cached, 'r'); - $this->preview = null; - if ($stream) { - $image = new \OC_Image(); - $image->loadFromFileHandle($stream); - $this->preview = $image->valid() ? $image : null; - - $this->resizeAndCrop(); - fclose($stream); - } + $this->getCachedPreview($fileId, $cached); } if (is_null($this->preview)) { - $preview = null; - - $previewProviders = \OC::$server->getPreviewManager()->getProviders(); - foreach ($previewProviders as $supportedMimeType => $providers) { - if (!preg_match($supportedMimeType, $this->mimeType)) { - continue; - } - - foreach ($providers as $closure) { - $provider = $closure(); - if (!($provider instanceof \OCP\Preview\IProvider)) { - continue; - } - - \OC_Log::write('core', 'Generating preview for "' . $file . '" with "' . get_class($provider) . '"', \OC_Log::DEBUG); - - /** @var $provider Provider */ - $preview = $provider->getThumbnail($file, $maxX, $maxY, $scalingUp, $this->fileView); - - if (!($preview instanceof \OCP\IImage)) { - continue; - } - - $this->preview = $preview; - $this->resizeAndCrop(); - - $previewPath = $this->getPreviewPath($fileId); - $cachePath = $this->buildCachePath($fileId); - - if ($this->userView->is_dir($this->getThumbnailsFolder() . '/') === false) { - $this->userView->mkdir($this->getThumbnailsFolder() . '/'); - } - - if ($this->userView->is_dir($previewPath) === false) { - $this->userView->mkdir($previewPath); - } - - $this->userView->file_put_contents($cachePath, $preview->data()); - - break 2; - } - } + $this->generatePreview($fileId); } + // We still don't have a preview, so we generate an empty object which can't be displayed if (is_null($this->preview)) { $this->preview = new \OC_Image(); } @@ -588,18 +549,58 @@ class Preview { } /** - * resize, crop and fix orientation - * @return void + * Retrieves the preview from the cache and resizes it if necessary + * + * @param int $fileId fileId of the original image + * @param string $cached the path to the cached preview */ - private function resizeAndCrop() { + private function getCachedPreview($fileId, $cached) { + $stream = $this->userView->fopen($cached, 'r'); + $this->preview = null; + if ($stream) { + $image = new \OC_Image(); + $image->loadFromFileHandle($stream); + + $this->preview = $image->valid() ? $image : null; + + $maxX = (int)$this->getMaxX(); + $maxY = (int)$this->getMaxY(); + $previewX = (int)$this->preview->width(); + $previewY = (int)$this->preview->height(); + + if ($previewX !== $maxX && $previewY !== $maxY) { + $this->resizeAndStore($fileId); + } + + fclose($stream); + } + } + + /** + * Resizes, crops, fixes orientation and stores in the cache + * + * @param int $fileId fileId of the original image + */ + private function resizeAndStore($fileId) { + // Resize and store + $this->resizeAndCrop(); + // We save a copy in the cache to speed up future calls + $cachePath = $this->buildCachePath($fileId); + $this->userView->file_put_contents($cachePath, $this->preview->data()); + } + + /** + * resize, crop and fix orientation + * + * @param bool $max + */ + private function resizeAndCrop($max = false) { $image = $this->preview; - $x = $this->getMaxX(); - $y = $this->getMaxY(); - $scalingUp = $this->getScalingUp(); - $maxScaleFactor = $this->getMaxScaleFactor(); + + list($x, $y, $scalingUp, $maxScaleFactor) = $this->getResizeData($max); if (!($image instanceof \OCP\IImage)) { - \OC_Log::write('core', '$this->preview is not an instance of \OCP\IImage', \OC_Log::DEBUG); + \OCP\Util::writeLog('core', '$this->preview is not an instance of OC_Image', \OCP\Util::DEBUG); return; } @@ -617,6 +618,7 @@ class Preview { } } + // The preview already has the asked dimensions if ($x === $realX && $y === $realY) { $this->preview = $image; return; @@ -639,7 +641,7 @@ class Preview { if (!is_null($maxScaleFactor)) { if ($factor > $maxScaleFactor) { - \OC_Log::write('core', 'scale factor reduced from ' . $factor . ' to ' . $maxScaleFactor, \OC_Log::DEBUG); + \OCP\Util::writeLog('core', 'scale factor reduced from ' . $factor . ' to ' . $maxScaleFactor, \OCP\Util::DEBUG); $factor = $maxScaleFactor; } } @@ -649,11 +651,13 @@ class Preview { $image->preciseResize($newXSize, $newYSize); + // The preview has been upscaled and now has the asked dimensions if ($newXSize === $x && $newYSize === $y) { $this->preview = $image; return; } + // One dimension of the upscaled preview is too big if ($newXSize >= $x && $newYSize >= $y) { $cropX = floor(abs($x - $newXSize) * 0.5); //don't crop previews on the Y axis, this sucks if it's a document. @@ -666,6 +670,7 @@ class Preview { return; } + // One dimension of the upscaled preview is too small and we're allowed to scale up if (($newXSize < $x || $newYSize < $y) && $scalingUp) { if ($newXSize > $x) { $cropX = floor(($newXSize - $x) * 0.5); @@ -698,10 +703,160 @@ class Preview { $image = new \OC_Image($backgroundLayer); $this->preview = $image; + return; } } + /** + * Returns data to be used to resize a preview + * + * @param $max + * + * @return array + */ + private function getResizeData($max) { + if (!$max) { + $x = $this->getMaxX(); + $y = $this->getMaxY(); + $scalingUp = $this->getScalingUp(); + $maxScaleFactor = $this->getMaxScaleFactor(); + } else { + $x = $this->configMaxX; + $y = $this->configMaxY; + $scalingUp = false; + $maxScaleFactor =1; + } + + return [$x, $y, $scalingUp, $maxScaleFactor]; + } + + /** + * Returns the path to a preview based on its dimensions and aspect + * + * @param int $fileId + * + * @return string + */ + private function buildCachePath($fileId) { + $maxX = $this->getMaxX(); + $maxY = $this->getMaxY(); + + $previewPath = $this->getPreviewPath($fileId); + $previewPath = $previewPath . strval($maxX) . '-' . strval($maxY); + if ($this->keepAspect) { + $previewPath .= '-with-aspect'; + } + $previewPath .= '.png'; + + return $previewPath; + } + + /** + * @param int $fileId + * + * @return string + */ + private function getPreviewPath($fileId) { + return $this->getThumbnailsFolder() . '/' . $fileId . '/'; + } + + /** + * Asks the provider to send a preview of the file of maximum dimensions + * and after saving it in the cache, it is then resized to the asked dimensions + * + * This is only called once in order to generate a large PNG of dimensions defined in the + * configuration file. We'll be able to quickly resize it later on. + * We never upscale the original conversion as this will be done later by the resizing operation + * + * @param int $fileId fileId of the original image + */ + private function generatePreview($fileId) { + $file = $this->getFile(); + $preview = null; + + $previewProviders = \OC::$server->getPreviewManager()->getProviders(); + foreach ($previewProviders as $supportedMimeType => $providers) { + if (!preg_match($supportedMimeType, $this->mimeType)) { + continue; + } + + foreach ($providers as $closure) { + $provider = $closure(); + if (!($provider instanceof \OCP\Preview\IProvider)) { + continue; + } + + \OCP\Util::writeLog( + 'core', 'Generating preview for "' . $file . '" with "' . get_class($provider) + . '"', \OCP\Util::DEBUG + ); + + /** @var $provider Provider */ + $preview = $provider->getThumbnail( + $file, $this->configMaxX, $this->configMaxY, $scalingUp = false, $this->fileView + ); + + if (!($preview instanceof \OCP\IImage)) { + continue; + } + + $this->preview = $preview; + $previewPath = $this->getPreviewPath($fileId); + + if ($this->userView->is_dir($this->getThumbnailsFolder() . '/') === false) { + $this->userView->mkdir($this->getThumbnailsFolder() . '/'); + } + + if ($this->userView->is_dir($previewPath) === false) { + $this->userView->mkdir($previewPath); + } + + // This stores our large preview so that it can be used in subsequent resizing requests + $this->storeMaxPreview($previewPath); + + break 2; + } + } + + // The providers have been kind enough to give us a preview + if ($preview) { + $this->resizeAndStore($fileId); + } + } + + /** + * Stores the max preview in the cache + * + * @param string $previewPath path to the preview + */ + private function storeMaxPreview($previewPath) { + $maxPreview = false; + $preview = $this->preview; + + $allThumbnails = $this->userView->getDirectoryContent($previewPath); + // This is so that the cache doesn't need emptying when upgrading + // Can be replaced by an upgrade script... + foreach ($allThumbnails as $thumbnail) { + $name = rtrim($thumbnail['name'], '.png'); + if (strpos($name, 'max')) { + $maxPreview = true; + break; + } + } + // We haven't found the max preview, so we create it + if (!$maxPreview) { + // Most providers don't resize their thumbnails yet + $this->resizeAndCrop(true); + + $maxX = $preview->width(); + $maxY = $preview->height(); + $previewPath = $previewPath . strval($maxX) . '-' . strval($maxY); + $previewPath .= '-max.png'; + $this->userView->file_put_contents($previewPath, $preview->data()); + } + } + /** * @param array $args */ @@ -791,30 +946,4 @@ class Preview { $preview->deleteAllPreviews(); } - /** - * @param int $fileId - * @return string - */ - private function buildCachePath($fileId) { - $maxX = $this->getMaxX(); - $maxY = $this->getMaxY(); - - $previewPath = $this->getPreviewPath($fileId); - $preview = $previewPath . strval($maxX) . '-' . strval($maxY); - if ($this->keepAspect) { - $preview .= '-with-aspect'; - } - $preview .= '.png'; - - return $preview; - } - - - /** - * @param int $fileId - * @return string - */ - private function getPreviewPath($fileId) { - return $this->getThumbnailsFolder() . '/' . $fileId . '/'; - } } diff --git a/tests/lib/preview.php b/tests/lib/preview.php index 2a6761403f..003ecedb65 100644 --- a/tests/lib/preview.php +++ b/tests/lib/preview.php @@ -48,6 +48,77 @@ class Preview extends TestCase { parent::tearDown(); } + public function testIsMaxSizeWorking() { + // Max size from config + $maxX = 1024; + $maxY = 1024; + + \OC::$server->getConfig()->setSystemValue('preview_max_x', $maxX); + \OC::$server->getConfig()->setSystemValue('preview_max_y', $maxY); + + // Sample is 1680x1050 JPEG + $sampleFile = '/' . $this->user . '/files/testimage.jpg'; + $this->rootView->file_put_contents($sampleFile, file_get_contents(\OC::$SERVERROOT.'/tests/data/testimage.jpg')); + $fileInfo = $this->rootView->getFileInfo($sampleFile); + $fileId = $fileInfo['fileid']; + + $largeX = 1920; + $largeY = 1080; + $preview = new \OC\Preview($this->user, 'files/', 'testimage.jpg', $largeX, $largeY); + + $this->assertEquals($preview->isFileValid(), true); + + // There should be no cached copy + $isCached = $preview->isCached($fileId); + + $this->assertNotEquals(\OC\Preview::THUMBNAILS_FOLDER . '/' . $fileId . '/' . $maxX . '-' . $maxY . '-max.png', $isCached); + $this->assertNotEquals(\OC\Preview::THUMBNAILS_FOLDER . '/' . $fileId . '/' . $maxX . '-' . $maxY . '.png', $isCached); + $this->assertNotEquals(\OC\Preview::THUMBNAILS_FOLDER . '/' . $fileId . '/' . $largeX . '-' . $largeY . '.png', $isCached); + + // The returned preview should be of max size + $image = $preview->getPreview(); + + $this->assertEquals($image->width(), $maxX); + $this->assertEquals($image->height(), $maxY); + + // The max thumbnail should be created + $maxThumbCacheFile = '/' . $this->user . '/' . \OC\Preview::THUMBNAILS_FOLDER . '/' . $fileId . '/' . $maxX . '-' . $maxY . '-max.png'; + + $this->assertEquals($this->rootView->file_exists($maxThumbCacheFile), true); + + // A preview of the asked size should not have been created + $thumbCacheFile = \OC\Preview::THUMBNAILS_FOLDER . '/' . $fileId . '/' . $largeX . '-' . $largeY . '.png'; + + $this->assertEquals($this->rootView->file_exists($thumbCacheFile), false); + + // 2nd request should indicate that we have a cached copy of max dimension + $isCached = $preview->isCached($fileId); + $this->assertEquals(\OC\Preview::THUMBNAILS_FOLDER . '/' . $fileId . '/' . $maxX . '-' . $maxY . '.png', $isCached); + + // Smaller previews should be based on the cached max preview + $smallX = 50; + $smallY = 50; + $preview = new \OC\Preview($this->user, 'files/', 'testimage.jpg', $smallX, $smallY); + $isCached = $preview->isCached($fileId); + + $this->assertEquals(\OC\Preview::THUMBNAILS_FOLDER . '/' . $fileId . '/' . $maxX . '-' . $maxY . '.png', $isCached); + + // A small preview should be created + $image = $preview->getPreview(); + $this->assertEquals($image->width(), $smallX); + $this->assertEquals($image->height(), $smallY); + + // The cache should contain the small preview + $thumbCacheFile = '/' . $this->user . '/' . \OC\Preview::THUMBNAILS_FOLDER . '/' . $fileId . '/' . $smallX . '-' . $smallY . '.png'; + + $this->assertEquals($this->rootView->file_exists($thumbCacheFile), true); + + // 2nd request should indicate that we have a cached copy of the exact dimension + $isCached = $preview->isCached($fileId); + + $this->assertEquals(\OC\Preview::THUMBNAILS_FOLDER . '/' . $fileId . '/' . $smallX . '-' . $smallY . '.png', $isCached); + } + public function testIsPreviewDeleted() { $sampleFile = '/'.$this->user.'/files/test.txt'; @@ -96,25 +167,6 @@ class Preview extends TestCase { $this->assertEquals($this->rootView->is_dir($thumbCacheFolder), false); } - public function testIsMaxSizeWorking() { - - $maxX = 250; - $maxY = 250; - - \OC_Config::setValue('preview_max_x', $maxX); - \OC_Config::setValue('preview_max_y', $maxY); - - $sampleFile = '/'.$this->user.'/files/test.txt'; - - $this->rootView->file_put_contents($sampleFile, 'dummy file data'); - - $preview = new \OC\Preview($this->user, 'files/', 'test.txt', 1000, 1000); - $image = $preview->getPreview(); - - $this->assertEquals($image->width(), $maxX); - $this->assertEquals($image->height(), $maxY); - } - public function txtBlacklist() { $txt = 'random text file';