Skip to content
This repository was archived by the owner on Nov 14, 2018. It is now read-only.

[gallery thumbnails] orientation doesn't work #1568

Closed
ghost opened this issue Dec 18, 2013 · 14 comments
Closed

[gallery thumbnails] orientation doesn't work #1568

ghost opened this issue Dec 18, 2013 · 14 comments
Labels

Comments

@ghost
Copy link

ghost commented Dec 18, 2013

It seems that for Gallery thumbnails OC_Image is being created from filehandle so exif_read_data in getOrientation() fails because it doesn't know file full path, which is why gallery shows thumbnails with incorrect orientation.

I'm running OC 6.0.

I managed to add a quick hack which seemingly fixes it, but I'm not sure if there are possible side effects:

In apps/gallery/lib/thumbnail.php, lines 68-70:

//      $handle = $this->view->fopen($imagePath, 'r');
//      $this->image = new \OCP\Image($handle);
//      fclose($handle);
        $this->image = new \OCP\Image(null);
        $this->image->load($this->view->getLocalFile($imagePath));

(original code is commented out)

Sorry for the lack of proper diff.

@ghost
Copy link
Author

ghost commented Dec 18, 2013

Well, I have also managed to make a workaround for publicly shared images not rotating properly, I'll add a full diff against OC 6.0.0a for both issues here:

diff -ur /var/www/owncloud/apps/files_sharing/public.php /home/owncloud/apps/files_sharing/public.php
--- /var/www/owncloud/apps/files_sharing/public.php 2013-12-14 23:47:38.000000000 +0400
+++ /home/owncloud/apps/files_sharing/public.php    2013-12-18 21:31:21.826422347 +0400
@@ -133,6 +133,26 @@
            OC_Files::get($dir, $file, $_SERVER['REQUEST_METHOD'] == 'HEAD');
        }
        exit();
+   } else if (isset($_GET['preview'])) {
+       $local = \OC\Files\Filesystem::getLocalFile($path);
+
+       $rotate = false;
+       if (is_callable('exif_read_data')) { //don't use OCP\Image here, using OCP\Image will always cause parsing the image file
+           $exif = @exif_read_data($local, 'IFD0');
+           if (isset($exif['Orientation'])) {
+               $rotate = ($exif['Orientation'] > 1);
+           }
+       }
+       if ($rotate) {
+           $image = new OCP\Image($local);
+           $image->fixOrientation();
+           $image->show();
+       } else { //use the original file if we dont need to rotate, saves having to re-encode the image
+           OC_Files::get($dir, $file, $_SERVER['REQUEST_METHOD'] == 'HEAD');
+       }
+
+       exit();
+
    } else {
        OCP\Util::addScript('files', 'file-upload');
        OCP\Util::addStyle('files_sharing', 'public');
@@ -210,7 +230,7 @@
            $list->assign('files', $files);
            $list->assign('baseURL', OCP\Util::linkToPublic('files') . $urlLinkIdentifiers . '&path=');
            $list->assign('downloadURL',
-               OCP\Util::linkToPublic('files') . $urlLinkIdentifiers . '&download&path=');
+                   OCP\Util::linkToPublic('files') . $urlLinkIdentifiers . '&download&path=');
            $list->assign('isPublic', true);
            $list->assign('sharingtoken', $token);
            $list->assign('sharingroot', $basePath);
@@ -251,6 +271,7 @@
            // Show file preview if viewer is available
            if ($type == 'file') {
                $tmpl->assign('downloadURL', OCP\Util::linkToPublic('files') . $urlLinkIdentifiers . '&download');
+               $tmpl->assign('previewURL', OCP\Util::linkToPublic('files') . $urlLinkIdentifiers . '&preview');
            } else {
                $tmpl->assign('downloadURL', OCP\Util::linkToPublic('files')
                                        .$urlLinkIdentifiers.'&download&path='.urlencode($getPath));
diff -ur /var/www/owncloud/apps/files_sharing/templates/public.php /home/owncloud/apps/files_sharing/templates/public.php
--- /var/www/owncloud/apps/files_sharing/templates/public.php   2013-12-14 23:47:38.000000000 +0400
+++ /home/owncloud/apps/files_sharing/templates/public.php  2013-12-18 21:15:40.273342665 +0400
@@ -74,7 +74,7 @@
        <?php else: ?>
            <?php if (substr($_['mimetype'], 0, strpos($_['mimetype'], '/')) == 'image'): ?>
                <div id="imgframe">
-                   <img src="<?php p($_['downloadURL']); ?>" />
+                   <img src="<?php p($_['previewURL']); ?>" />
                </div>
            <?php elseif (substr($_['mimetype'], 0, strpos($_['mimetype'], '/')) == 'video'): ?>
                <div id="imgframe">
diff -ur /var/www/owncloud/apps/gallery/lib/thumbnail.php /home/owncloud/apps/gallery/lib/thumbnail.php
--- /var/www/owncloud/apps/gallery/lib/thumbnail.php    2013-12-14 23:47:55.000000000 +0400
+++ /home/owncloud/apps/gallery/lib/thumbnail.php   2013-12-18 21:00:03.912609950 +0400
@@ -65,9 +65,11 @@
        if (!$this->view->file_exists($imagePath)) {
            return;
        }
-       $handle = $this->view->fopen($imagePath, 'r');
-       $this->image = new \OCP\Image($handle);
-       fclose($handle);
+//     $handle = $this->view->fopen($imagePath, 'r');
+       $this->image = new \OCP\Image(null);
+       $this->image->load($this->view->getLocalFile($imagePath));
+//     fclose($handle);
+
        if ($this->image->valid()) {
            $this->image->fixOrientation();
            if ($square) {

@karlitschek
Copy link
Contributor

@georgehrke Can you have a look?

@georgehrke
Copy link
Contributor

@georgehrke Can you have a look?

Sure, I should find some time later today

@josh4trunks
Copy link

The proposed changes here to 'apps/gallery/lib/thumbnail.php' would probably also fix the thumbnail rotation in the below issue.
#1555

This change would also be needed to fix rotation with php5.5.
#1462 (comment)

@georgehrke
Copy link
Contributor

@gothfox Do you agree to release your code changes under the terms of the MIT License?

@josh4trunks
Copy link

@georgehrke Can you make sure to include the below change which fixes the php5.5 rotation problem? This is a different issue then the issue mentioned in this thread. Thanks

#1462 (comment)

@pascalBokBok
Copy link

Could it be because the thumbnails are converted to PNG? That would probably destroy the rotation hints. see #1590

@pascalBokBok
Copy link

Due to bug #1590 the thumbnails are converted to PNG and thus does not contain EXIF information[1] about any rotation.

[1] http://stackoverflow.com/questions/9542359/does-png-contain-exif-data-like-jpg

@PorkCharsui79
Copy link

Hello, I'm also experiencing rotated thumbnails in the picture gallery. The mouse-over slide-show shows the pics in the correct orientation and if you view the files individually they're also correct. Only the thumbnails of the pictures themselves are rotated. Before I upgraded to OC6 the thumbnail orientation was correct. My setup:
debian wheezy
owncloud 6.0.1
nginx
php5.4+xcache+php5-fpm
mariadb

Does anyone have any idea what can be done about this. My php and ajax skillz aren't... just aren't. But I can apply a patch :P.

@pascalBokBok
Copy link

@pornstarsui
Then take a look at the patch for #1590 and provide icewind1991 some feedback. I believe the rotation does not happen because the rotation info is lost in conversion to PNG.

@PorkCharsui79
Copy link

Thanks Pascal... I didn't know the thumbnails were png files.

@pascalBokBok
Copy link

@pornstarsui No wonder. The thumbnails keep the .jpg extension in the filename. It seems to be an error - it doesn't make sense to convert JPEGs to PNG. They are two very different file formats. JPEG are great for Photos - PNG are great for graphics with large blocks of exactly the same color.
For instance.
I have a photo: 0,75MB as JPEG. If I save as a PNG it's 5,8MB. If I then paint over the whole image(5616x3744) with a green color, the PNG filesize is 69KB and the JPEG is 241KB.

@PVince81
Copy link
Contributor

PVince81 commented Mar 6, 2014

@gothfox can you please make a PR with your changes ?
This will make it easier for everyone to review and test them.
Thanks 😄

@josh4trunks
Copy link

I believe this issue has been solved in master for owncloud/gallery apps thumbnails. Picture extension, type, and orientation are all preserved for the gallery app's thumbnails.

  • Tested on FreeBSD 9.2, PHP 5.5, SQLite, with Encrytion both On and Off

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

7 participants