[Android] StorageProvider Fixes

[ksooo]

Fixes: #19023 #19307

DISCLAIMER: I’m not an Android dev, and it might be not the best fix or even nonsense, but for me it definitely fixes the problem of SMB shares mounted on the NVidia Shield TV at system level not appearing in the list of removable drives of Kodi.

DISCLAIMER: I’m not an Android dev, but with the help of @koying I was able to track down and fix the problems

• Android (Nvidia Shield) smb shares do not appear in Kodi 19 #19023 : SMB shares mounted on the NVidia Shield TV at system level not appearing in Kodi
• Kodi 19.0.0 for Android displays hexadecimal volume names (disk drives) #19307 : Removable volumes labeled with UID instead of the actual volume label

Some details I found out while debugging:

• On Android, removable volumes are obtained using CAndroidStorageProvider::GetRemovableDrives
• This method first tries to obtain the volumes using JNI: CJNIStorageManager::getStorageVolumes eventually calls Java StorageManager.getStorageVolumes.
• This call fails (Java throws an exception) on current master/Matrix branches, at least on my Shield, but the code has a fallback to obtain the volumes the Linux way using /proc/mounts.
• This seems to work; the Samba shares are reported by Android, but the current filter for valid devices does not include a proper entry to detect SMB devices (starting with a double slash) and the fstype filter is missing (cifs).

First commit fixes the fallback to support SMB mounts.
Second commit fixes the non working primary code path, fixing both SMB mounts not appearing and volume labels.

I runtime-tested the changes on my Shields, with both SMB mounts and USB sticks.

@koying by any chance, if you are around, does this fix make any sense to you?

[koying]

@ksooo Makes sense. The fallback is to parse "/proc/mounts", so whatever it takes ;)
Not sure what you mean by "getStorageVolumes is failing" (for SMB or in general?) but SMB mounting is specific to Shield, and there is no support for it in Android proper, afaik (but I'm a bit behind on droid API's ;) )

[ksooo]

@koying The call to getStorageVolumes is always throwing an exception.

[ksooo]

The real question is: Why does StorageManager.getStorageVolumes throw an exception? The answer most probably is the key for the actual fix.

[koying]

If you attach a log showing the exception, I might maybe point you in the right direction...

[ksooo]

Should I see the exception in logcat?

[koying]

Mmm... Not currently. The exception is just eaten away.

You might add a xbmc_jnienv()->ExceptionDescribe(); before

xbmc/xbmc/platform/android/storage/AndroidStorageProvider.cpp

Line 127 in a80d129

xbmc_jnienv()->ExceptionClear();

(assuming this is where the exception occurs; probably best is to just add it to all exception catches, before the ExceptionClear())

I think that will write the exception description to logcat ;)

[ksooo]

@koying:

04-06 16:31:26.548 22913 23053 W System.err: java.lang.NoSuchMethodError: no non-static method "Landroid/os/storage/StorageManager;.getStorageVolumes()[Landroid/os/storage/StorageVolume;"

[koying]

Yeah, getStorageVolumes returns a java list, not an array.
You can basically mimic CJNIList<CJNIRouteInfo> CJNILinkProperties::getRoutes, both in androidjni and in kodi

[ksooo]

Thanks, @koying

Yeah, getStorageVolumes returns a java list, not an array.

Which means that this code never worked since xbmc/libandroidjni@ee5cc10#diff-532e8e365b57f7770256c95b1c9f1e1d07d171295158dd4fa29b85e09ccbe889

Interesting. :-/

[koying]

Very likely not. My original code was based on an unofficial API, iirc.
When Google made it official, they renamed it AND changed the return value by a list rather than an array: https://android.googlesource.com/platform/frameworks/base/+/c02bfae%5E!/

[ksooo]

@koying the libandroidjni fixes you suggested are here: xbmc/libandroidjni#22
I will add a corresponding commit to this PR here once you have approved that PR.

[Tolriq]

Time for a small reminder: https://developer.android.com/distribute/best-practices/develop/target-sdk
API 30 for November (https://developer.android.com/training/data-storage#scoped-storage) This time there won't be the magical flag on Android 11.

[ksooo]

Time for a small reminder

Let's hope that a new Android maintainer arises soon, otherwise you could write as many reminders as you like or none at all, with the same result.

[Tolriq]

@ksooo Do not ban from forums those who can and helped ;)

BTW You merged way too fast getStorageVolumes is API 24 minimum (https://developer.android.com/reference/android/os/storage/StorageManager#getStorageVolumes()) , Kodi is api 21 minimum, this will crash on Android 5 5.1 and 6.

[koying]

Shouldn't crash. It will trap and fallback, as it was doing before...

[Tolriq]

Don't know how it's called externally but the code previously checked if the function was present before call, now it blindly calls. (https://github.com/xbmc/libandroidjni/pull/22/files) Should have a version check here or at call site with doc on API limitation. Furthermore it probably now remove the function that worked on those API levels (assuming it was not broken for other reasons)

[ksooo]

You merged way too fast

No panic. Thanks for pointing this out. No idea whether it actually would crash, but it is absolutely no problem to add an API check before calling. I will add this.

[ksooo]

xbmc/libandroidjni#23

[ksooo]

@koying could you please take a look at xbmc/libandroidjni#23 ?