Overhaul image upload - Githubissues

vickychijwani commented 7 years ago

The current image upload code suffers from several potential issues because of the unnecessary, forced conversion to JPEG, although no user has come forward with these yet:

The image may be of lower quality than the original due to JPEG compression
The user may (reasonably) not expect their image to be messed with on the way up
The file may get blown up in size depending on the source encoding and so on
Some images may not be converted correctly, for any reason - if Ghost can work with it, we shouldn't get in the way
It slows down the image upload process by performing this extra conversion

Given that the JPEG compression is 100% unnecessary and was only done for early ease-of-implementation, it should be removed now. The app should directly upload whatever file was chosen to Ghost. This blog post dives deep into how to do exactly that, and the code seems robust enough to handle different Android versions and so on.

Related: #154, #153

vickychijwani commented 7 years ago

I've ended up using a hybrid approach that first tries to get the image path and upload it directly, and failing that falls back to the old JPEG conversion approach. This should be more robust than both methods while preserving data integrity as far as possible. See the commit message for more details.

vickychijwani commented 7 years ago

This deserves another look after further helpful advice from StackOverflow. It should be possible to simply get an InputStream from the picked Uri and write it to a File without decoding it into a Bitmap first, that way we preserve the original encoding exactly and don't have to mess around with Uri => path conversion in a hacky way.

NOTE: Retrofit 1.x supports uploading directly from an InputStream, Retrofit 2.x also does.

Also fix #119 to avoid SecurityException on Android 6.0+.

RcrdBrt commented 7 years ago

Images inserted into articles from the Quill app end up displaying the broken image icon (see below) after some time (undefined amount of time ranging from 2 days to 2 weeks on avg) for no apparent reason. Images uploaded from the ghost web admin interface from a desktop PC do not suffer this.

broken image icon ---> r390n

I saw on the 1.5.4 release you addressed some image upload problems. Is that fix related to the problem I described above and this issue?

By the way many thanks Vicky, your project is awesome and deserves eternal respect (plus official Ghost Foundation baking in my opinion).

vickychijwani commented 7 years ago

@RcrdBrt if the URL for the image is still correct after the broken image shows up, and if the same image is still correctly displayed on the Ghost web interface, then the likely culprit is your internet connection. Could you try going to App Settings, clear the cache, and open the post again with a stable connection? At least that would confirm the problem.

RcrdBrt commented 7 years ago

The url is fine when the image breaks, it was the first thing that came to my mind to troubleshoot this. Those images are displayed correctly for like 2 weeks and then suddenly the Ghost blog fails to "render" them both on mobile and desktop. I downloaded the actual files uploaded from the app via rsync and I can open and watch them just fine (also their thumbnails if it can help) from a random image viewer or even Google Chrome.

vickychijwani commented 7 years ago

I see. Can you try the cache clearing workaround I described above?

RcrdBrt commented 7 years ago

Sure, I'll let you know if it still happens.

Thanks!

vickychijwani commented 7 years ago

Oh you know what, I just read your previous comment carefully and noticed you said it fails to render even on desktop. That's... interesting, and strange.

I think the new update (v1.5.5) might fix the issue, because before this I used to convert (i.e., decode and re-encode) the image to a JPEG (even if it was already a JPEG), but now I upload the image exactly in its original format. Let me know if you still see it after updating to 1.5.5 and uploading some new images.

RcrdBrt commented 7 years ago

Yeah, it fails on desktop too. I thought that maybe there was some shared cache system I wasn't aware of and I was trying to do your suggestion anyway. I read about the JPEG not-necessary compression your app does. I hope that is the issue, but I don't know since whatsapp shared images are already jpeg and even those ones get broken after some time if I upload them with Quill.

vickychijwani commented 7 years ago

but I don't know since whatsapp shared images are already jpeg and even those ones get broken after some time

In older app versions, even if an image was already a JPEG, it would still have been re-encoded - so the new version might fix it.

vickychijwani / quill

Overhaul image upload #155