Skip to content
This repository has been archived by the owner on Feb 9, 2018. It is now read-only.

Overhaul image upload #155

Closed
vickychijwani opened this issue Feb 1, 2017 · 10 comments
Closed

Overhaul image upload #155

vickychijwani opened this issue Feb 1, 2017 · 10 comments

Comments

@vickychijwani
Copy link
Owner

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 added a commit that referenced this issue Feb 5, 2017
…cannot

always be converted to a filesystem path.

As explained here - http://stackoverflow.com/a/39490576/504611 - a URI from
ACTION_PICK cannot reliably be converted to a URI. It is easy to observe this
when, for example, picking a file via Clean File Manager or a Google Photos pic
that is not already on the device.

The hybrid approach should be more reliable while preserving data integrity as
far as possible - this is not perfect but better than the alternatives.

References #155.
vickychijwani added a commit that referenced this issue Feb 5, 2017
…cannot

always be converted to a filesystem path.

As explained here - http://stackoverflow.com/a/39490576/504611 - a URI from
ACTION_PICK cannot reliably be converted to a URI. It is easy to observe this
when, for example, picking a file via Clean File Manager or a Google Photos pic
that is not already on the device.

The hybrid approach should be more reliable while preserving data integrity as
far as possible - this is not perfect but better than the alternatives.

References #155.
@vickychijwani
Copy link
Owner Author

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 vickychijwani reopened this Feb 6, 2017
@vickychijwani
Copy link
Owner Author

vickychijwani commented Feb 6, 2017

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
Copy link

RcrdBrt commented Feb 19, 2017

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
Copy link
Owner Author

@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
Copy link

RcrdBrt commented Feb 19, 2017

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
Copy link
Owner Author

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

@RcrdBrt
Copy link

RcrdBrt commented Feb 19, 2017

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

Thanks!

@vickychijwani
Copy link
Owner Author

vickychijwani commented Feb 19, 2017

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
Copy link

RcrdBrt commented Feb 19, 2017

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
Copy link
Owner Author

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.

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

No branches or pull requests

2 participants