Instant uploads based on watch for new files in camera folder by davivel · Pull Request #1783 · owncloud/android

davivel · 2016-08-25T08:25:56Z

Fixes #6, and means instant uploads... :

... work in Android 7;
... work with more camera apps.

New requisites for camera apps will be:

store pictures and videos in public storage (there's not much sense in doing other way);
store pictures and videos in same folder (this can be extended in the future if needed)

BUGS & IMPROVEMENTS

Camera MX does not upload videos Instant uploads based on watch for new files in camera folder #1783 (comment) [FIXED]
tmp file in video uploads "only wifi" with no connection Instant uploads based on watch for new files in camera folder #1783 (comment) [FIXED]
Changing from 4G to only wifi, the last file is not retried Instant uploads based on watch for new files in camera folder #1783 (comment) [FIXED]
Instant uploads with default camera do not work in Samsung Note 4 with Android 6 Instant uploads based on watch for new files in camera folder #1783 (comment) [WONT]. Moved to Instant uploads in Samsung Galaxy Note4 with default camera #1786

mention-bot · 2016-08-25T08:26:01Z

@davivel, thanks for your PR! By analyzing the annotation information on this pull request, we identified @przybylski, @tobiasKaminsky and @jabarros to be potential reviewers

davivel · 2016-08-25T08:27:28Z

@jesmrec , ready to test.

@nasli and @owncloud/android-developers , code reviews are welcome.

davivel · 2016-08-25T08:40:39Z

src/com/owncloud/android/db/PreferenceManager.java

+    private static final String PREF__INSTANT_PICTURE_UPLOAD_PATH = "instant_upload_path";
+    private static final String PREF__INSTANT_VIDEO_UPLOAD_PATH = "instant_video_upload_path";
+    private static final String PREF__INSTANT_UPLOAD_BEHAVIOUR = "prefs_instant_behaviour";
+    private static final String PREF__INSTANT_UPLOAD_SOURCE = "instant_upload_source_path";   // NEW - not saved yet


Self note: instant_upload_source_path is saved, the comment is wrong.

nasli · 2016-08-25T09:29:27Z

Wow, awesome refactor and fast-timing for Android7 ! Let's release it! 👍

davivel · 2016-08-25T09:32:08Z

For the record: Travis fails getting build dependencies from Google, code is OK. Let's see with a retry,

jesmrec · 2016-08-25T11:10:30Z

@davivel i have noticed this:

The "Camera MX" app does not upload the videos taken into its folder `...DCIM/CameraMX' . The following log is print at the moment the video starts recording:

08-25 13:00:37.256 11051-11068/com.owncloud.android W/InstantUploadsObserver: Camera app saved an empty or temporary file, ignoring VIDEO_20160825_130036.mp4

With this app the photos works correctly. I have tested other apps and works properly with videos and photos. Checked with empty and non-empty folder.

Perhaps is a corner case that only happens with this app, but having a look can be a good idea :)

Checked in Android7 (Huawei 6P), Android6 (Nexus9) and Android5 (Nexus5)

davivel · 2016-08-25T11:20:23Z

I'll have a look to see if we can do something fast. Video recording might become a problem. Seems that every camera app handles file its-very-own-way. I tested with Google Camera and OpenCamera, both save the video in different ways (OpenCamera keeps it simple).

Let me debug a bit and cross your fingers.

Any case involving video uploads should be blocked meanwhile.

davivel · 2016-08-25T11:32:14Z

MOG. The camera is "writing&closing" the same file twice after creating. Probably it's doing:

start recording: create new file, then close it empty;
stop recording: open same file, save video and close file;

We are not tracking that workflow. And it's a crappy one, since it's indistinguishable of updating a video file, what we didn't want to track.

I need to think a bit about this. Ideas are welcome.

Instant video was working for this app in Android 6 with ownCloud 2.1.0 ?

jesmrec · 2016-08-25T11:36:41Z

i'm not pretty sure but i think they did. Checking...

jesmrec · 2016-08-25T12:10:19Z

@davivel checked with 2.1.0 + Camera MX and works correctly. Videos & Photos are uploaded without error.

davivel · 2016-08-25T12:53:23Z

OK, seems I have a solution.

davivel · 2016-08-25T13:07:05Z

@jesmrec, should work now.

@nasli , you should give another look to code updated. All the changes are in file InstantUploadsObserver

jesmrec · 2016-08-25T13:17:31Z

With Android5 and Android4 (backwards compatibility)

Steps:

Enable instant uploads for videos with "only wifi"
Disable wifi
Take a video

Actual behaviour:

In uploads view, the video appears twice as pending to upload

Video_1234.mp4
Video_1234.mp4.tmp

Expected behaviour:

In uploads view, only one file related to the video appears in the queue

Nexus5 v5.0.1

davivel · 2016-08-25T13:18:45Z

Really? That .tmp never should have got here.

What camera app are you using in that device?

jesmrec · 2016-08-25T13:22:51Z

i used default camera.

davivel · 2016-08-25T13:33:22Z

OK, that was stupid from my side.

Fixed now also, @jesmrec

jesmrec · 2016-08-26T08:50:26Z

Bug (low severity)

Steps:

In a device with a SIM card, enable the instant uploads with only wifi
Switch the wifi off (4G is up)
Take several photos/videos -> pending of wifi connection
Switch the wifi on

Actual behaviour

All the photos/videos are uploaded but the last one, that have to be tapped manually to upload

Expected behaviour

All the photos are uploaded

From plane mode to wifi mode -> works fine

Tested with Android 7. Lower Android versions, work correctly

davivel · 2016-08-26T09:05:52Z

Taking a look.

jesmrec · 2016-08-26T09:12:06Z

Bug (only in samsung note 4 Android 6)

Enable instant uploads
Set the default camera folder with the default camera
Take some photos and videos

Actual behaviour

Nothing is uploaded

Expected behaviour

Content is uploaded

davivel · 2016-08-26T09:14:17Z

That's worse. Checking.

davivel · 2016-08-26T11:34:03Z

As discussed offline: this last bug is very, very specific of the OS version + camera app + camera app configuration, maybe also model, and there is no way we can fix this with a limited effort. Let's open another issue for it.

jesmrec · 2016-08-29T10:27:59Z

@davivel working fine. Approved.

Now we should be aware of posible differences related to device/software vendors that can cause exceptions like #1783 (comment).

Great job ;)

… in camera folder

…curacy of undesired renaming check

…ces that network is really available

davivel · 2016-08-29T10:45:23Z

Rebasing.

Great job you :D

davivel added this to the 2.2.0 milestone Aug 25, 2016

davivel added the Android_N label Aug 25, 2016

davivel reviewed Aug 25, 2016
View reviewed changes

jesmrec mentioned this pull request Aug 25, 2016

[PR 1783] Instant uploads from observer owncloud/QA#296

Closed

davivel force-pushed the instant_upload_from_observer branch from be3b833 to b098be8 Compare August 26, 2016 06:31

davivel mentioned this pull request Aug 26, 2016

OC Android App on Android 7.0 - Instant Picture Upload Not Triggering #1784

Closed

jesmrec added the Approved by qa label Aug 29, 2016

davivel added 12 commits August 29, 2016 12:44

Moved RecursiveFolderObserver into services.observer package

d00548f

Implement instant uploads from camera based on watching for new files…

e0930d4

… in camera folder

RIP InstantUploadBroadcastReceiver. You were a good soldier

abf30ab

Add preference to set camera folder for instant uploads

c45b7af

Fixed NPE and updated comments, from self-review

b7f0e86

Delete commented code

097ec89

Consider only created & MODIFIED files before requesting instant upload

0aab901

Delay final checks on file candidate for instant upload for better ac…

6c8cbd9

…curacy of undesired renaming check

Prevent memory overflow...

d333d0e

Don't upload when new file in camera folder is not a picture or a video

9a90e6a

Prevent double path separator in instant uploads

b90d52d

Delay request to retry uploads when Wifi is recovered to increase cha…

99352d4

…ces that network is really available

davivel force-pushed the instant_upload_from_observer branch from 98a052b to 99352d4 Compare August 29, 2016 10:44

davivel merged commit 5534fe0 into master Aug 29, 2016

davivel deleted the instant_upload_from_observer branch August 29, 2016 10:54

davivel modified the milestones: 2.1.1, 2.2.0 Aug 29, 2016

davivel mentioned this pull request Sep 2, 2016

[WIP] Extend instant upload #1650

Closed

Comments

Conversation

davivel commented Aug 25, 2016 • edited by jesmrec Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mention-bot commented Aug 25, 2016

Uh oh!

davivel commented Aug 25, 2016

Uh oh!

davivel Aug 25, 2016

Choose a reason for hiding this comment

Uh oh!

nasli commented Aug 25, 2016

Uh oh!

davivel commented Aug 25, 2016

Uh oh!

jesmrec commented Aug 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davivel commented Aug 25, 2016

Uh oh!

davivel commented Aug 25, 2016

Uh oh!

jesmrec commented Aug 25, 2016

Uh oh!

jesmrec commented Aug 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davivel commented Aug 25, 2016

Uh oh!

davivel commented Aug 25, 2016

Uh oh!

jesmrec commented Aug 25, 2016

Uh oh!

davivel commented Aug 25, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jesmrec commented Aug 25, 2016

Uh oh!

davivel commented Aug 25, 2016

Uh oh!

jesmrec commented Aug 26, 2016

Bug (low severity)

Steps:

Actual behaviour

Expected behaviour

Uh oh!

davivel commented Aug 26, 2016

Uh oh!

jesmrec commented Aug 26, 2016

Bug (only in samsung note 4 Android 6)

Actual behaviour

Expected behaviour

Uh oh!

davivel commented Aug 26, 2016

Uh oh!

davivel commented Aug 26, 2016

Uh oh!

jesmrec commented Aug 29, 2016

Uh oh!

davivel commented Aug 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

davivel commented Aug 25, 2016 •

edited by jesmrec

Loading

jesmrec commented Aug 25, 2016 •

edited

Loading

jesmrec commented Aug 25, 2016 •

edited

Loading

davivel commented Aug 25, 2016 •

edited

Loading