nextcloud / android

📱 Nextcloud Android app
https://play.google.com/store/apps/details?id=com.nextcloud.client
GNU General Public License v2.0
4.26k stars 1.76k forks source link

Android 10: Media scanner can't be used on custom paths #6150

Closed tobiasKaminsky closed 10 months ago

tobiasKaminsky commented 4 years ago

Steps to reproduce

  1. use auto upload on Android 10
  2. see: Adding image to media scanner failed: java.lang.IllegalArgumentException: Primary directory (invalid) not allowed for content://media/external_primary/images/media; allowed directories are [DCIM, Pictures]

Path is: /storage/emulated/0/Android/media/com.nextcloud.client/nextcloud/tobi@10.0.2.2%2Fnc/InstantUpload/Camera/2020/05/IMG_20200526_072938.jpg

@sirambd do you have an idea?

tobiasKaminsky commented 4 years ago

For now I created a PR to not let media scanner throw an exception: https://github.com/nextcloud/android/pull/6151

sirambd commented 4 years ago

@tobiasKaminsky It's just that the MediaStore needs to add the scanned item in a specific folder of its own from android Q (API 29).

If we want to add an element to the table mediaStore, we must follow this architecture that google wants. It's for Android 10 and above

tobiasKaminsky commented 4 years ago

So this does mean that no image within our folders can be found any more…? This is indeed a big step back from Android :/

tobiasKaminsky commented 4 years ago

@sirambd do you have an idea? :-)

simonspa commented 4 years ago

I was also got by this today after upgrading to a phone with Android 10. This is quite a bummer because it basically eliminates the way the app is supposed to work on many use cases, such as:

all because media is not discoverable anymore by other apps.

How do other apps with similar functionality deal with this issue?

simonspa commented 4 years ago

@AndyScherzinger do you maybe have an idea?

svenb1234 commented 4 years ago

According to the comments above there are folders for each media type. Do other apps use them?

If a user wants to find the files on the Android file system, these upper level folders are certainly more discoverable then the current folders five levels below the user's root.

simonspa commented 4 years ago

That's true - but how would you map an arbitrary complex folder structure from your cloud account into some folders in which also other apps dump their data?

svenb1234 commented 4 years ago

I am no expert, but having a Nextcloud folder in each of the media folders, e.g Pictures, with the structure known to the user from the file view of the web interface seems understandable. In e.g. the Google Fotos App that Nextcloud Folder would be visible as well.

It is true though that this would spread the Nextcloud files across several folders for Audio, Video, Images and Download for the rest.

AndyScherzinger commented 4 years ago

@AndyScherzinger do you maybe have an idea?

as of now, unfortunately no :/

simonspa commented 4 years ago

Trying to read some Android documentation: is this something that could be solved by adapting scoped storage? (keep in mind that I have never written a single loc for Android but just read the docs...)

Relevant reads:

simonspa commented 4 years ago

...and this here seems to be very similar (if not identical) to our problem: https://stackoverflow.com/questions/59637302/android-10-q-the-mediastore-and-mediaplayer-playfromuri

simonspa commented 4 years ago

Sorry for the spam, just reading up on this. Google mentions in their blog post on scoped storage (https://android-developers.googleblog.com/2019/04/android-q-scoped-storage-best-practices.html) that now data fall in these categories:

Storing shared media files. For apps that handle files that users expect to be sharable with other apps (such as photos) and be retained after the app has been uninstalled, use the MediaStore API. There are specific collections for common media files: Audio, Video, and Images. For other file types, you can store them in the new Downloads collection. To access files from the Downloads collection, apps must use the system picker. Storing app-internal files. If your app is designed to handle files not meant to be shared with other apps, store them in your package-specific directories. This helps keep files organized and limit file clutter as the OS will manage cleanup when the app is uninstalled. Calls to Context.getExternalFilesDir() will continue to work.

For us, the problem seems to be that we fall in between the cracks here: we have files we expect to share with other apps but which we also expect to go away if the app is uninstalled.

AndyScherzinger commented 4 years ago

Trying to read some Android documentation: is this something that could be solved by adapting scoped storage?

Maybe @ArisuOngaku can provide some insight for this while I am aware you are very busy these days, so no pressure!

ashpieboop commented 4 years ago

Sorry, but I actually don't know much about scoped storage past its existence. All the resources linked here about it seem to be complete.

My understanding is that all user's files are to be shared media files. It doesn't matter if they stay on the device after the app is uninstalled; it could actually be unexpected to the user for them to vanish. App-internal files are files that don't belong to the user; i.e. cache, temp files, (config files?)...

Also, if I'm not mistaken, the app should sort file types by shared media type (i.e music, pictures...) (Downloads by default).

I hope this is somewhat useful.

sirambd commented 4 years ago

We have two solutions: We can separate media files in different folders Or keep the existing logic, by creating a NextCloud sub-folder in the download folder. For MediaStore, the download folder can contain everything.

And for info google uses the 2nd solution

Then we can just offer the user to choose just the type of storage from the settings. (sdcard, telephone, etc...). Only the ones that are available.

We propose this solution only on android 10 and above, but for the rest we keep the existing solution. @tobiasKaminsky

simonspa commented 4 years ago

Somehow I don't get Google's design decision behind all of this. In principle, the Android/media was a good place for this sort of usage, now they deliberately broke it and drive people to circumvent it by just moving things to the Downloads folder?

The only issue I see with this is that people might accidentally delete NC files since they are in the same folder as their GIFs from the web.

JorisBodin commented 4 years ago

We can separate media files in different folders Or keep the existing logic, by creating a NextCloud sub-folder in the download folder. For MediaStore, the download folder can contain everything.

The first solution is much too long to implement. And is not implemented by google.

I agree with the second solution. On a phone, it's file browsing.

simonspa commented 4 years ago

I guess users would then expect that files moved into that folder are uploaded - which would mean the 2-way-sync feature would become a bit more urgent?

sirambd commented 4 years ago

@simonspa No we don't have to put everything in the download folder, we can separate them like images, in the pictures folder, video in movies, etc...

But I think it's better if everything is just in a NextCloud folder from the download folder, so NextCloud files will be separated from other online download files.

My solution is a sub dir in download folder.

That way we always keep the synchronization bidirectional.

simonspa commented 4 years ago

@sirambd I understood that and I agree - but it remains that currently - to my knowledge - the NC android app does not support 2-way sync. I.e. if I store something in a NC folder, the app does not pick that up automatically and uploads it to the server, this only works through the auto-upload feature.

I fear that this will lead to some confusion if all NC data is just housed in a subfolder of "Downloads" but I also don't see a better solution...

tobiasKaminsky commented 4 years ago

@simonspa No we don't have to put everything in the download folder, we can separate them like images, in the pictures folder, video in movies, etc...

we cannot do this…imagine if you download a folder "vacation" with 200 images and 20 videos and 2 pdfs. Then you browse to /download/nextcloud/vacation/ and would only see 2 pdfs, but somewhere else images and videos. Furthermore it would be very hard to keep track where all the files are.

@sirambd I understood that and I agree - but it remains that currently - to my knowledge - the NC android app does not support 2-way sync. I.e. if I store something in a NC folder, the app does not pick that up automatically and uploads it to the server, this only works through the auto-upload feature.

That is true, but also true for current situation. If you put a new file in an existing NC folder nothing happens…

--> we have already the possibility to change storage path, so it would be "enough" to add downloads to it and users can then switch?

sirambd commented 4 years ago

@tobiasKaminsky Yes, that's it :-D

That way, we'll just have a NextCloud folder in the download folder with all NextCloud files, images, videos, pdf, etc...

simonspa commented 4 years ago

--> we have already the possibility to change storage path, so it would be "enough" to add downloads to it and users can then switch?

As a first quick fix probably yes - unless there also needs to be a change how the mediaStore is notified.

sirambd commented 4 years ago

--> we have already the possibility to change storage path, so it would be "enough" to add downloads to it and users can then switch?

As a first quick fix probably yes - unless there also needs to be a change how the mediaStore is notified.

Not necessary.

AndyScherzinger commented 4 years ago

--> we have already the possibility to change storage path, so it would be "enough" to add downloads to it and users can then switch?

I'd also say that would be fine for now while it could probably be discussed if this is then a Android-10-only option or if we allow it for older Android versions too. Don't really see a reason not to except the testing efforts :/ What do you think @sirambd @tobiasKaminsky

tobiasKaminsky commented 4 years ago

Hm. I tried it in #6329 and hardcoded it to /sdcard/Download. It seems that a new image is added to MediaStore, but still it is not shown in Gallery app.

sirambd commented 4 years ago

Hm. I tried it in #6329 and hardcoded it to /sdcard/Download. It seems that a new image is added to MediaStore, but still it is not shown in Gallery app.

@tobiasKaminsky I've given you the pertinent comments as to why it might not work https://github.com/nextcloud/android/pull/6329#discussion_r444011875

tobiasKaminsky commented 4 years ago

You referenced the whole commit by me? What exactly is not correct in your opinion?

sirambd commented 4 years ago

I just updated my message, I hadn't made the right connection. But if not, I've listed everything https://github.com/nextcloud/android/pull/6329#discussion_r444011875

tobiasKaminsky commented 4 years ago

Thanks :+1: This seems to work, I will finish the PR.

sirambd commented 4 years ago

Thanks 👍 This seems to work, I will finish the PR.

Oh, good, you're welcome. 😁

tobiasKaminsky commented 4 years ago

I found another way to do this, without the need to change the location: #6352 Let us see if this also leads to vanishing ringtones like described in #5039 and then choose which PR we want to use.

simonspa commented 3 years ago

Any chance this could be revisited? I believe the fix is already available by @tobiasKaminsky in #6329 (and an alternative that was deemed not functional in #6352 - was this sure? The fix seems much smaller and less invasive...)

joshtrichards commented 10 months ago

I think this one was addressed by #10353 long ago.