Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop instant uploads in favor of automatic (non-instant) camera uploads #2028

Closed
9 tasks done
davivel opened this issue Sep 5, 2017 · 29 comments
Closed
9 tasks done
Assignees
Milestone

Comments

@davivel
Copy link
Contributor

davivel commented Sep 5, 2017

#From the feedback we got from #1829 and #1860, we know that we can't trust in the current implementation of instant uploads to grant that all the picture we'll be eventually uploaded.

At this moment we have tried all the things possible, in our knowledge, to upload the files "instantly", i.e: as soon as the pictures are taken.

We need to forget the idea of immediately and focus in reliability. Most of the feedback we have is from people that are much more concerned about having all there pictures effectively pushed to the server side than about the time needed to get it.

The problem of the current implementation is that the file observers we register for changes in the camera folder "live" in a background service that we try to keep alive forever. If that service is killed for any reason, the observers "die" and pictures are not detected anymore until the service is restarted. The restart is done when the app is opened again, or when the device is rebooted.

In the past, the death of the service didn't happen often. But from Android 4.4, Google is silently killing "immortal" services more and more often with each Android version. With Android 8 Google finally is documenting that services in background will not be allowed to live more than a limited time.

We need to go for a total rewrite of the detection of pictures, that will be based in the next key points:

  1. Schedule a job that syncs the camera folder periodically if auto uploads are enabled.
  2. Every time the job is triggered it should check the contents of the camera folder to find all the pictures that are not uploaded to the server. The job itself needs to compare the pictures in the local folder with the pictures that are already uploaded -- it cannot trust in being notified from the system per picture.
  3. The job will upload new pictures as new files to the server; if there are pictures already know but the app that were not successfully uploaded, the job will resume / retry those uploads too.

There are many other details that be improved on the road, but these points address the reliability problem we have in the current implementation.


TASKS:

  • Dive inside the current code to understand it better
  • Create new job to:
    • Check content of local camera folder.
    • Check content of upload folder.
    • Compare folders content and upload those images that don't exist in the server.
    • Retry previous unsuccessful uploads.
  • Upload only new pictures/videos (if user deletes a picture/video from the remote folder, it shouldn't be uploaded again)
  • Handle only-wifi option
  • Clean up previous implementation

BUGS & IMPROVEMENTS

  • (1) Crash in Android 8 when the ownCloud app is in background and tries to enqueue an upload with the camera file. [FIXED]
  • (2) UI keeps stuck during camera uploads sync if there are too many files to check when starting the app, job is being executed in the main thread, so move it to a another one. [FIXED]
  • (3) Crash in Android 8 when the ownCloud app is in background and tries to retry an upload. [FIXED]
  • (4) Retries with cellular network connection [FIXED]
@davivel davivel added this to the 2.5.1 milestone Sep 5, 2017
@davivel
Copy link
Contributor Author

davivel commented Sep 5, 2017

cc @michaelstingl , @jesmrec , @davigonz

@michaelstingl
Copy link
Contributor

Renaming this feature sounds reasonable. I think it's easier to communicate with a broad audience, if we coordinate with the release of the iOS app, where it's also not really "instant". /cc @pmaier1

@janipewter
Copy link

Ensuring 100% of photos are uploaded eventually is more important to me than 75% of photos being uploaded instantly and 25% being missed.

The Dropbox camera auto upload feature worked perfectly before I switched to Owncloud, is it possible for you to do things the way they do? I have disabled the power saving for Nextcloud client in Android and the auto upload seems relatively reliable for now, but I'd rather have the 100% reliability that Dropbox offered.

@hodyroff
Copy link

I think I would stick with the name but define it - keep in mind that the folder is called Instand Upload as well. And very much agree that 100% is most important.

@davigonz
Copy link
Contributor

davigonz commented Nov 2, 2017

Ensuring 100% of photos are uploaded eventually is more important to me than 75% of photos being uploaded instantly and 25% being missed.

@janipewter Totally agree

Coming back to the matter of choosing a name for this feature, from an user point of view, I would prefer something like "Camera uploads" , which is self explanatory and avoids entering in a discussion about wether uploads should be performed in 5 minutes, 10 or an hour.

I think I would stick with the name but define it

@hodyroff If we keep the "instant uploads" name, where could we explain that uploads are not instant? In the app with a warning? In the app manual?

IMHO, use the "Instant uploads" name could create misunderstandings, since users are going to expect that uploads start "instantly", i.e: as soon as the pictures or videos are taken.

And about "Auto uploads" , it is too generic.

Maybe some users that have taken part in other issues related to instant uploads want to give their opinion about the feature naming.

What do you think? @janipewter , @jensd0e , @fishstew , @M0ses , @kigero

CC / @michaelstingl @jesmrec @nasli

@jesmrec
Copy link
Collaborator

jesmrec commented Nov 2, 2017

@davigonz that's a good idea. But, taking in account the recent contribution #2059, we should think carefully how to call each feature. We could call both the same, but i mean this is confusing since they will work in a different way.

@davigonz
Copy link
Contributor

davigonz commented Nov 3, 2017

@jesmrec Yes, I see your point and they should be named differently. The feature to upload a picture directly from the camera could be implemented by using a new button with the text "Upload from camera" that appears when pressing the floating action button. See this mockup I included here: #2027 (comment)

On the other hand, the "Camera uploads" feature would continue appearing in the Settings app menu, like the previous "Instant uploads"

@jensd0e
Copy link

jensd0e commented Nov 3, 2017

I'd vote for "Camera uploads".
What it looks like right now (Version 2.5.0):


Instant uploads

Instant picture uploads
Instantly upload pictures taken by camera

Instant video uploads
Instantly upload videos recorded by camera


I'd just remove everything with "Instant/Instantly":


Camera uploads

Picture uploads
Upload pictures taken by camera

Video uploads
Upload videos recorded by camera


"Upload" just explains, what I'd want to happen: get my photos/videos uploaded to my ownCloud. :-)

@nasli
Copy link

nasli commented Nov 6, 2017

I will rather call it auto uploads, take into account that could be possible to upload pictures not just for the camera (at least for iOS)

@davigonz
Copy link
Contributor

davigonz commented Nov 9, 2017

Since testing this feature has a certain complexity due to the great bunch of involved devices, we are going to include it in a beta version previous to the final 2.5.1 release. Follow up: #2063

CC / @michaelstingl @jesmrec

@janipewter
Copy link

janipewter commented Nov 9, 2017

Auto upload is working great for me. Thanks guys for all of your hard work. It would be nice if "upload failed" notifications could be removed once the files have uploaded successfully though. I take quite a lot of photos in places with bad/no network coverage so uploads fail quite often on the first attempt, but then successfully complete later on, and the "upload failed" notifications stays in the notification tray until I clear it manually.

@davigonz
Copy link
Contributor

davigonz commented Nov 10, 2017

BUG (1) [FIXED]

Steps to reproduce

1.- Enable camera uploads for pictures/videos
2.- Exit ownCloud app (without killing it)
3.- Take a picture
4.- Wait a bit (15 minutes at most). During this period, Android start limiting ownCloud background services.

Current behaviour

The ownCloud app crashes

Expected behaviour

The ownCloud app performs the upload properly

Device & version

Nexus 6P with Android 8

@davigonz
Copy link
Contributor

davigonz commented Nov 13, 2017

BUG (2) [FIXED]

Steps to reproduce

1.- Enable camera uploads for pictures/videos, setting as source folder one with a lot of files.
2.- Kill the app
3.- Open the app

Current behaviour

The screen keeps white during several seconds

Expected behaviour

The files view appears faster

Device & version

Nexus 6P with Android 8

@davigonz
Copy link
Contributor

davigonz commented Nov 13, 2017

BUG (3) [FIXED]

Steps to reproduce

1.- Enable auto uploads for pictures/videos
2.- Exit ownCloud app
3.- Take several pictures/videos
4.- Turn off wifi
5.- Wait several minutes (10). During this period, Android should have limited ownCloud background services.
6.- Turn on wifi
7.- Wait a bit (15 minutes at most)

Current behaviour

The ownCloud app crashes

Expected behaviour

The ownCloud app retries the upload properly

Device & version

Nexus 6P with Android 8

@davigonz
Copy link
Contributor

davigonz commented Nov 13, 2017

IMPROVEMENT (4)

Steps to reproduce

1.- Turn off wifi internet connection and turn on cellular one
2.- Enable auto uploads for pictures/videos with the option "Upload pictures via wifi only" disabled
3.- Take a picture
4.- Wait till the upload starts
5.- Turn off cellular internet connection while upload is in progress. The upload should be cancelled.
6.- Turn on cellular internet connection

Current behaviour

Upload is not retried

Expected behaviour

The upload is retried, even with cellular internet connection

Device & version

Nexus 6P with Android 8

@davigonz davigonz reopened this Nov 13, 2017
@davigonz
Copy link
Contributor

Ups, wrong button 😌

@davigonz
Copy link
Contributor

davigonz commented Nov 16, 2017

Hi all @M0ses, @houseofbugs, @bkraul, @smmilani and @jensd0e, @janipewter , @fishstew , @kigero, @d98ama , @erwinfolmer.

We have just released the 2.5.1 beta version, which includes the new Camera uploads feature, that is replacing the previous Instant Uploads feature.

To use it, you just have to select a camera folder and the pictures and videos contained there will be automatically uploaded to the folder you select in a period of 15 minutes at most, focusing on reliability instead of immediacy and avoiding battery draining caused by excessive checking of the folder.

You can find this beta on Google Play Store and it will be on F-Droid too, in the next days.

Try it out and give us your feedback by commenting in this issue, thank you so much!

@sesipod
Copy link

sesipod commented Nov 28, 2017

I have tested this using my Samsung Galaxy S8+ SM-G955U and the default camera app & sd card DCIM location ( /storage/B6BF-89F1/DCIM/Camera ).
Android Version: 7.0
Build: NRD90M.G55USQU1AQK3

The upload is working about once every 5min or sometimes sooner. Thank you for getting this going and I will start following this post and testing on my S8+ :)

@davigonz
Copy link
Contributor

The upload is working about once every 5min or sometimes sooner. Thank you for getting this going and I will start following this post and testing on my S8+ :)

Great news, thank you so much for your feedback, if you have any suggestion or question do not hesitate to ask us

@sesipod
Copy link

sesipod commented Nov 29, 2017

@davigonz - So one issue that I see and this has happened 2 times already since yesterdays post.
The beta version of the app will loose my password to my owncloud server and uploads will fail. I re-enter my password and it recycles the login page and asks me again for it. The only way that I can fix this is to uninstall the app and reinstall it. I have tried just to clear the apps Cache/Storage but that doesn't work.
On the Stable release Owncloud (paid) app the account stays logged in and so does CardDav-sync free.

I will run with just the Beta version installed for now and see if the issue clears up....

Owncloud Beta - V 2.5.1-beta. 1
Owncloud - V 2.5.0
CardDav-sync V 0.4.21

@jensd0e
Copy link

jensd0e commented Nov 30, 2017

@sesipod I hijacked another thread I opened for a lightly different problem, since I have the same problem as you. Maybe a mod should spawn a new issue? See: #2030
That makes it hard to give feedback to this issue. ;-)

@davigonz
Copy link
Contributor

davigonz commented Nov 30, 2017

So one issue that I see and this has happened 2 times already since yesterdays post.
The beta version of the app will loose my password to my owncloud server and uploads will fail. I re-enter my password and it recycles the login page and asks me again for it. The only way that I can fix this is to uninstall the app and reinstall it. I have tried just to clear the apps Cache/Storage but that doesn't work.

Thank you for your feedback @sesipod but I think that the problem you describe is not related to camera uploads, can you please read the issue that @jensd0e included above (#2030) and see if the problem is the same? If so, add a comment there and if not, please open a new issue with the detailed steps and try to answer these questions:

  1. Do you perform any specific action before beta app loses the password or is it totally random?
  2. When you are redirected to login view and have to reenter the password, any error message appears?

Thanks

@jensd0e
Copy link

jensd0e commented Nov 30, 2017

@davigonz

  1. No. It's random, but takes sometimes many hours before this happens.
  2. No. I send a logfile to @jesmrec. But It's only a small excerpt showing only the time, when I'm redirected.

@sesipod
Copy link

sesipod commented Dec 1, 2017

@davigonz & @jensd0e

This is random issue... I reinstall the app the only way I can login once the issue happens. Then use it to move a file or two around then close out of the app. At some point later it will loose my credentials ( I notice this because picture uploads will fail and I get a notification.) So I reopen the app put in my password (it still has the server and username filled in). The login page will go away like a successful login but push me right back to the login screen. No errors at all.

@davigonz davigonz changed the title Drop instant uploads in favor of automatic (non-instant) uploads Drop instant uploads in favor of automatic (non-instant) camera uploads Jan 11, 2018
@davigonz
Copy link
Contributor

davigonz commented Jan 12, 2018

It's important for you to know that the new Camera uploads feature, which is going to replace the previous Instant uploads feature, needs to be enabled and configured, even if you had previous Instant Uploads feature enabled since is not going to be handled in the same way for the app.

SO, all the Instant Uploads configuration will be deleted after upgrading to 2.6.0, but you can easily enable the new Camera uploads feature and test it.

@jesmrec
Copy link
Collaborator

jesmrec commented Jan 12, 2018

Feature approved and finished. Let's enjoy the new camera uploads!!!

@arnwas
Copy link

arnwas commented Jul 7, 2018

This does not work on my Samsung S7 Edge with Oreo. Both from SD card nor internal memory

@jesmrec
Copy link
Collaborator

jesmrec commented Jul 9, 2018

@arnwas could you report here?: #2249

@tmctrain
Copy link

tmctrain commented Dec 5, 2020

Hello @ALL.
Sorry to continue this thread in the end of 2020, could not find new informations on this issues.

Running into troubles with new phone and owncloud sync.
Samsung A20e 32GB intern, 128GB microSD, Raspbery Owncloud 10.1, App 2.15.3
Pictures stored at /sdcard/DCIM/Camera
I'm trying to achieve auto upload of new pictures in this directory.
How can I set an sdcard folder to get automatically checked and uploaded?

Best regards.

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

No branches or pull requests