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

Support Android 10, atleast for now #786

Merged
merged 2 commits into from
Jan 29, 2020

Conversation

GeoZac
Copy link
Contributor

@GeoZac GeoZac commented Dec 18, 2019

Lot of the open issues are about issues on Android 10, namely storage issues and the seekbar on notification.

The notification seekbar is addressed by this commit

The storage issue is related to Scoped Storage, opting out of which is done by this commit. This is only a temporary solution. Better solution.

Also, another method to address the issues, is to revert to targeting SDK version of 28.

neobuddy89 and others added 2 commits December 18, 2019 20:08
Signed-off-by: Pranav Vashi <neobuddy89@gmail.com>
@GeoZac
Copy link
Contributor Author

GeoZac commented Dec 23, 2019

@VidiviciVeni Can you check if issues reported by you are fixed in this apk ?

@VidiviciVeni
Copy link

VidiviciVeni commented Dec 23, 2019

Hi @GeoZac,

Thank you for the suggestion -- I'm not a programmer, but it does seem like there is some kind of OS(Android)-wide - read/write restriction 'feature' with some recent update of Android that's keeping playlists from being able to be edited, outside of the official music playing system app, GPM. This might be one of the issues you're referencing in the OP.

One other thing I noted is that playlist edits in GPM, which work as-expected and hold through system restarts, for some reason do not carry over to playlist updates in third-party music-playing apps like Phonograph, etc. The playlists seem essentially frozen in time regardless of updates outside (GPM) or within (Phono) the app.

Is this apk download a link to an Android apk reversion or a Phonograph app update, tho? There are no details, just a direct-to-download-and-install link, which makes me a little uncomfortable. Surely you understand.

@GeoZac
Copy link
Contributor Author

GeoZac commented Dec 23, 2019

@VidiviciVeni The apk is a debug version of Phonograph with the two commits, addressed in this pull request, on the top of the original app.

I am currently running a build of Phonograph with the two changes, which is fixing the issues for me, and I would like to make sure the issues are solved with the changes I've proposed.

If the issues, you have reported are not present in this version of Phonograph, it would more reason for the developers to review the proposed changes and merge them into the app, if required.

Hope this clears your queries.

@VidiviciVeni
Copy link

@GeoZac,

I just downloaded your debugged apk of Phonograph and was able to replicate the playlist issues I'm having, unfortunately.

Using your debugged version, I took some songs and tried to add them to both the 'Favorites' list as well as another custom list I made, and they were added, as expected. However, like before, they couldn't be removed from the playlist after adding, and restarting the phone reset all of the playlists to back before any kind or amount of edits.

@paolovalerdi
Copy link
Contributor

I think it would be better to look at getting rid of the current playlists management that relies on the MediaStore. Instead, it'll be worth if we all could think of a custom database schema that allows us to create and edit playlists. I think it wouldn´t be hard to implement if we do it using the Room library.

@VidiviciVeni
Copy link

@paolovalerdi is this issue being experienced by anyone and everyone using Phonograph? It's hard to know of it's just me, or just Pixel 2 XL owners, or Android 10 users, et al?

@paolovalerdi
Copy link
Contributor

@VidiviciVeni Every app that relies of the MediaStore for playlists have this issue and others, like playlists disappearing from time to time

@mardous
Copy link
Contributor

mardous commented Dec 31, 2019

@GeoZac As I have informed myself, the SAF API could work to recover access to music files, but the main problem is without a doubt the playlists, I think that the idea of ​​@paolovalerdi is complicated but not impossible

@enricocid
Copy link

enricocid commented Jan 21, 2020

To make the app fully compatible with Android 10 you need to get content uri from id. You can use content uri for playback.
Path, i.e. data column, is not accessible in Android 10 running on real devices (on emulator it doesn't lead to crashes so it is impossible to test scoped storage).
To extract info from the tracks using the meta data retriever or media extractor it is possible to use file descriptor like in this example.

So, I didn't need to opt out scoped storage

    fun getContentUri(audioId: Long): Uri {
        return ContentUris.withAppendedId(
            MediaStore.Audio.Media.EXTERNAL_CONTENT_URI,
            audioId
        )
    }

    fun getAudioFileDescriptor(
        contentUri: Uri,
        contentResolver: ContentResolver
    ): ParcelFileDescriptor? {
        val readOnlyMode = "r"
        return contentResolver.openFileDescriptor(contentUri, readOnlyMode)
    }

You can easily convert to java.

I also have problems retrieving the music from the device. The query returns different results after some days of usage. Seems solved after having followed the official docs about mediastore querying.

For more info about media management: https://developer.android.com/training/data-storage/shared/media#java

Anyway my app is not implementing tags editing or other complex features and the Android 10 changes will be a problem starting from Android R where it will not be possible to opt out scoped storage anymore

@kabouzeid
Copy link
Owner

@enricocid on my Google Pixel Android 10, the data column is still accessible

@kabouzeid kabouzeid merged commit aa97988 into kabouzeid:master Jan 29, 2020
@kabouzeid
Copy link
Owner

Thanks @GeoZac

@enricocid
Copy link

enricocid commented Jan 29, 2020

@enricocid on my Google Pixel Android 10, the data column is still accessible

even without having to opt out of scoped storage? I noticed this behaviour also when emulating a pixel device with the AVD.

I don't know if the same applies to other manufacturers.

I received some reports from users using other manufacturers devices running Android 10 like this one. Some users were unable to even boot the app, reporting crashes during the query that I was able to temporarily fix by adding android:requestLegacyExternalStorage="true" to the manifest.

I then implemented content uri and file descriptors and there are no problems now for Android 10.
:)

@kabouzeid
Copy link
Owner

Yes I can read the data column with and without android:requestLegacyExternalStorage="true".

Btw, we already use content uri for playback.

@GeoZac GeoZac deleted the fixesforupstream branch January 30, 2020 04:58
@enricocid
Copy link

enricocid commented Feb 1, 2020

I think it would be better to look at getting rid of the current playlists management that relies on the MediaStore. Instead, it'll be worth if we all could think of a custom database schema that allows us to create and edit playlists. I think it wouldn´t be hard to implement if we do it using the Room library.

I'm experiencing this issue too (thought it was fixed, but it isn't). After some days of usage MediaStore query returns different results. This happens also on Phonograph and Shuttle and all media player querying MediaStore at boot.
I think this is the problem behind playlists disappearing.

So I started implementing a db as You suggested where to store results from MediaStore.

It works great and it is not very difficult to handle. That's the solution together with a preference to rebuild it.
Gonna test the implementation in the upcoming days and I will share the changes here so You can profit (anyway, you need to convert to java).

@paolovalerdi
Copy link
Contributor

@enricocid Brilliant! I'm currently learning dbs and I really couldn't come with a proper schema to work with lmao, I'll take the time to convert it to java when you're done ;)

@enricocid
Copy link

enricocid commented Feb 9, 2020

These are the changes related to the db

enricocid/Music-Player-GO@460a38f

enricocid/Music-Player-GO@a4234be

For the music data class an alternative is to let Room take care of ID (just add null when creating a new music object)
https://github.com/enricocid/Music-Player-GO/blob/master-database-playlist/project/app/src/main/java/com/iven/musicplayergo/models/Music.kt

Saving songs is pretty easy, but it does not solve the issue for devices storing music on external storage.

I'm actually able to reproduce the issue when the device is locked and unlocked back again (but it does not happen everytime you lock the device, it is pretty random/occasional).

For some reason the sdcard get unmounted and mounted back again when unlocking the screen (you also get notified in the status bar).
This behaviour forces the MediaStore to rebuild the media database assigning new _ID to the records. In fact, if You try to open the app as soon as this happens, You won't be able to query MediaStore as this process is slow, especially when You have many music files.

So, even if You store the music with their _IDs You won't be able to play them as _ID is changed meanwhile (happens after some hours) and so does contentUri.

A solution could be to store only certain info and then retrieve the music files _IDs updating playlists too, but again, what happens if the problem above appears? You won't be able to query _ID until the MediaStore has finished updating its records.

I think this is a very difficult problem to solve. At least for me, unfortunately I'm not holding advanced database skills.

I released the changes and I will create a new branch, meanwhile I will restore the previous implementation and I'll try to inform my users :)

@kabouzeid
Copy link
Owner

Android 11 will enforce this, and requestLegacyExternalStorage=true will no longer work.
Does anyone here has experience with implementing scoped storage?

Source: https://developer.android.com/preview/privacy

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

Successfully merging this pull request may close these issues.

None yet

7 participants