Fix imagerotate and move loadFromBase64() one up

[Niduroki]

imagerotate() with third parameter being -1 does not seem to work in PHP 5.5

loadFromBase64() one up, so debug-logs aren't spammed (as much) with urlencoded base64-data from loadFromFile() debug output.

Not sure whether the order in OC_Image->load() has to be the way it is now (base64-like file-paths?), but doing some short tests didn't seem like it.

[Niduroki]

@tanghus

[scrutinizer-notifier]

A new inspection was created.

[Niduroki]

This should be backported as well, imo. The same thing is happening in stable6 to me. @karlitschek

[ghost]

🚀 Test Passed. 🚀
Refer to this link for build results: https://ci.owncloud.org/job/pull-request-analyser/3059/

[tanghus]

Not tested but looks good.

[PVince81]

For reference, the php5.5 rotation issue was known: owncloud-archive/apps#1555

[tanghus]

I've hardly touched this class since I made it in in apparently 2011 so other reviewers are needed.
@bartv2 @butonic @icewind1991 and some "Johannes Willnecker" who I have no idea who is?

[PVince81]

@josh4trunks @bjalt can you help testing this ?

[josh4trunks]

I'll try it tonight. ill test with php5.5

[PVince81]

Excellent. Make sure to use the "command line" instructions in the green box because checking out the branch directly won't work until this PR is rebased onto master by @Kondou-ger

[PVince81]

I mean do this:

git fetch origin
git checkout -b imagerotate_and_loadbase64_fix origin/imagerotate_and_loadbase64_fix
git merge master

I'm testing on PHP 5.4 and PHP 5.3 now to make sure it still works.

• PHP 5.3
• PHP 5.4
• PHP 5.5

[PVince81]

Tested on openSUSE 13.1 with PHP 5.4.
Tested on CentOS 6.5 with PHP 5.3.

Images are rotated correctly in the thumbnails and gallery app.

👍

Let's wait for the feedback from @josh4trunks for the PHP 5.5 case.

[josh4trunks]

The commandline instructions and even just the latest master were giving me a white screen. It might be because my setup hosts owncloud in a subdirectory (eg. http://example.com/owncloud), or because my setup uses php-fpm. I just ended up applying the commit to 6.0.2

Tested on FreeBSD 10.0 with PHP 5.5

• In the 'Files' app, images are rotated correctly in thumbnails and fullscreen
• In the 'Pictures' app, images are rotated correctly in fullscreen but not as thumbnails (I assume thumbnails in the 'Pictures' app were fixed by another commit)

You need me to try something else? I'll fish around for a commit on thumbnails in the 'Pictures'/Gallery app.

[Niduroki]

php-fpm shouldn't be a problem, as I'm using that as well.

[PVince81]

@josh4trunks you need to run git submodule update when switching branches, also make sure your "apps" repository is up to date as well.

I tried again with PHP 5.4 and the pictures app rotates the thumbnails correctly, and also when viewing the pics they are rotated as well.

@josh4trunks when testing, please make sure to upload new pics because the old ones might be cached unrotated already.

[josh4trunks]

@Kondou-ger awesome, must be something else I did then.
@PVince81 OK, I'll see if I can get it going working with that command.
Everytime I test it is a fresh install with newly uploaded pictures.

[josh4trunks]

cd /usr/local/www git clone https://github.com/owncloud/core ./owncloud git submodule update --init git fetch origin git checkout -b imagerotate_and_loadbase64_fix origin/imagerotate_and_loadbase64_fix git merge master git clone https://github.com/owncloud/gallery ./apps/gallery chown -R www:www ../owncloud

Using the above commands to create a new development environment worked. I am getting the same behavior I earlier reported, picture rotation works in the files app's thumbnail and fullscreen view.

Rotation doesn't work in the 'Gallery' app's thumbnail view. This is because the thumbnail was converted to a PNG even though the filename still ends with JPG, this has been reported here owncloud-archive/apps#1568

I vote to merge this commit, it works for PHP 5.3-5, the gallery thumbnail rotation issue is unrelated.

[Niduroki]

@josh4trunks thumbnails are a different issue, it's the same as this: #7141 (comment)

[josh4trunks]

@Kondou-ger so you're it's an underlying issue with OCImage? Do you believe the JPG>PNG conversion is the cause (I assume the exif data is lost when converting to PNG), or do you think that is an unrelated issue?

[josh4trunks]

@Kondou-ger I just uploaded an EXIF rotated JPG as a profile picture. The resulting thumbnail is a PNG (which doesn't respect the EXIF rotation).

[Niduroki]

Thumbnails are converted to png? Didn't even know that. Might be helpful, might not be.
I tried rotating the image quite early, which didn't work, and I also tried rotating a test image with a simple load from data and then rotating (in a simple 10 line php file on its own), which didn't work either.

[josh4trunks]

Yup, really weird, thought the filename stays JPG. But downloading the resulting file shows it's a PNG. PNGs can't have associated EXIF data so either..

1. don't convert to PNG, convert everything to JPEG
2. factor in EXIF and rotate JPGs before converting to PNG
   I think option 1 is best because JPGs generally compress more and the quality shouldn't matter for a thumbnail.

[tanghus]

I haven't followed this thread closely, but from what I know about GD the problem is that unless you load the image from file, you have no idea of the format. AFAIRC that's why I decided to convert output as PNG unless the format was known. In loadFromBase64, loadFromResource there's no EXIF data available, so I guess the problem lays in the original loading of the image. If it's loaded from file any EXIF data conversions most be made then if the image is to be transformed in any way, otherwise the data is lost. I think loadFromData will still have metadata, but it has to be extracted from the data at load time.
You have possibly already discussed this, just wanna chip in with the little I remember from when I originally made the class - I have generally very little interest in image manipulation ;)

[josh4trunks]

Awesome, thanks for the clarification! So that still leaves us with the two options I mentioned earlier. @PVince81 you said thumbs (for JPGs with EXIF rotation) in the Gallery app worked for you? What happens if you upload a JPG with EXIF rotation as your profile picture?

[tanghus]

from when I originally made the class

The first non-static class in ownCloud btw 😄

[josh4trunks]

Awesome =] guess you can never get away from it now, lol

[tanghus]

Trying ;)

[PVince81]

I was actually testing with JPG files all the time.
@josh4trunks I have tried uploading a JPG avatar and it got rotated correctly.

Since this change doesn't break anything and seems to partially fix the thumbnail rotation in PHP 5.5, my thumbs up still apply 👍

This needs another thumbs up: @josh4trunks or @tanghus ?

[PVince81]

I just realized that I was using already rotated pictures... seems the Nexus 4 does not only set exif info but also saves the pictures already rotated...

I had another try with pics from a camera and here is the result

1. thumbnails in files app correctly rotated
2. picture correctly rotated when displayed full screen (slideshow mode)
3. gallery app: does not rotate image thumbnail ([gallery thumbnails] orientation doesn't work owncloud-archive/apps#1568)
4. gallery app: full screen pic is correctly rotated
5. profile picture: NOT rotated (Profile image not auto-rotated nor manually rotateable #7141 (comment))

So, if @josh4trunks had the same results on PHP 5.5. then it is already an improvement so we can get this merged once we get the second thumbs up 😄

[josh4trunks]

I've been for this getting merged, I just didn't know the procedure =]
👍

[josh4trunks]

Ok, so now to figure out if we should..

1. change thumbnails to JPG instead of PNG, (and try to keep exif rotation)
2. pre-rotate JPG before converting to PNG

Unless @tanghus has any qualms I've for 1

[PVince81]

@josh4trunks thanks. Going to merge this now.
For the JPG/PNG issue please follow up in owncloud-archive/apps#1568 😄

Edit: fixed issue number

[PVince81]

This one seems to be relevant as well: owncloud-archive/apps#1590