Add sliding window for the playback queue when in Android Auto mode.
Description
Adds a sliding window of 15 tracks (7 before, the current track, and 7 after), visible in the playback queue. The window slides with the current tack number, so if the first track is played, tracks 1-15 will show, if the last track is played the last track minus 15 will show, etc.
Motivation and Context
Android Auto limits the number of visible tracks to 20 when the vehicle is not in park. If you have a large playlist, it is no longer possible to use the queue to see the upcoming track names. This resolves the issue.
How Has This Been Tested?
Evaluated in-vehicle for several weeks.
Types of changes
-
New feature (non-breaking change which adds functionality)
Checklist
-
I have read the CONTRIBUTING section of the README document.
More Information
Mention: #1514 (closed)
Merge request reports
Activity
- Resolved by Nicolas Pomepuy
Depends on !765 (merged)
- Resolved by Robert Stone
- Resolved by Nicolas Pomepuy
- Resolved by Robert Stone
added 1 commit
- a5fbff12 - Add sliding window for the playback queue when in Android Auto mode. Threading...
added 1 commit
- 7438d76d - Add sliding window for the playback queue when in Android Auto mode. Threading...
added 1 commit
- 9be14388 - Add sliding window for the playback queue when in Android Auto mode.
- Resolved by Nicolas Pomepuy
added 3 commits
-
9be14388...8c991851 - 2 commits from branch
videolan:master
- 603b9386 - Add sliding window for the playback queue when in Android Auto mode.
-
9be14388...8c991851 - 2 commits from branch
enabled an automatic merge when the pipeline for 603b9386 succeeds
added Android Auto label
mentioned in issue #1514 (closed)
1085 1089 1086 1090 private fun updateMediaQueue() = lifecycleScope.launch(start = CoroutineStart.UNDISPATCHED) { 1087 1091 if (!this@PlaybackService::mediaSession.isInitialized) initMediaSession() 1092 artworkMap = HashMap<String, Uri>().also { 1093 val artworkToUriCache = HashMap<String, Uri>() 1094 for (media in playlistManager.getMediaList()) { Hey @rhstone, I get a weird crash on this line in the Play Store console.
Here is the stacktrace (line 1182 is corresponding to this line => 1094):
java.lang.NullPointerException: at org.videolan.vlc.PlaybackService$updateMediaQueue$1.invokeSuspend (PlaybackService.kt:1182) at org.videolan.vlc.PlaybackService$updateMediaQueue$1.invoke (Unknown Source:10) at kotlinx.coroutines.intrinsics.UndispatchedKt.startCoroutineUndispatched (Undispatched.kt:55) at kotlinx.coroutines.CoroutineStart.invoke (CoroutineStart.kt:111) at kotlinx.coroutines.AbstractCoroutine.start (AbstractCoroutine.kt:158) at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch (Builders.common.kt:56) at kotlinx.coroutines.BuildersKt.launch (Unknown Source:1) at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch$default (Builders.common.kt:49) at kotlinx.coroutines.BuildersKt.launch$default (Unknown Source:1) at org.videolan.vlc.PlaybackService.updateMediaQueue (PlaybackService.kt:1177) at org.videolan.vlc.PlaybackService.onPlaylistLoaded (PlaybackService.kt:856) at org.videolan.vlc.media.PlaylistManager.load (PlaylistManager.kt:150) at org.videolan.vlc.media.PlaylistManager.load$default (PlaylistManager.kt:128) at org.videolan.vlc.PlaybackService$load$2.invokeSuspend (PlaybackService.kt:1175) at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith (ContinuationImpl.kt:33) at kotlinx.coroutines.DispatchedTask.run (DispatchedTask.kt:106) at kotlinx.coroutines.EventLoop.processUnconfinedEvent (EventLoop.common.kt:69) at kotlinx.coroutines.DispatchedTaskKt.resumeUnconfined (DispatchedTask.kt:236) at kotlinx.coroutines.DispatchedTaskKt.dispatch (DispatchedTask.kt:161) at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume (CancellableContinuationImpl.kt:362) at kotlinx.coroutines.CancellableContinuationImpl.completeResume (CancellableContinuationImpl.kt:479) at kotlinx.coroutines.channels.AbstractChannel$ReceiveHasNext.completeResumeReceive (AbstractChannel.kt:939) at kotlinx.coroutines.channels.AbstractSendChannel.offerInternal (AbstractChannel.kt:56) at kotlinx.coroutines.channels.LinkedListChannel.offerInternal (LinkedListChannel.kt:29) at kotlinx.coroutines.channels.AbstractSendChannel.offer (AbstractChannel.kt:141) at kotlinx.coroutines.channels.ChannelCoroutine.offer (Unknown Source:2) at org.videolan.tools.KotlinExtensionsKt.safeOffer (KotlinExtensions.kt:118) at org.videolan.vlc.media.MediaUtils$SuspendDialogCallback.<init> (MediaUtils.kt:476) at org.videolan.vlc.media.MediaUtils.openList (MediaUtils.kt:303) at org.videolan.vlc.media.MediaUtils.openList$default (MediaUtils.kt:301) at org.videolan.vlc.viewmodels.mobile.VideosViewModel$play$1.invokeSuspend (VideosViewModel.kt:95) at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith (ContinuationImpl.kt:33) at kotlinx.coroutines.DispatchedTask.run (DispatchedTask.kt:106) at android.os.Handler.handleCallback (Handler.java:938) at android.os.Handler.dispatchMessage (Handler.java:99) at android.os.Looper.loop (Looper.java:236) at android.app.ActivityThread.main (ActivityThread.java:8037) at java.lang.reflect.Method.invoke (Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:656) at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:967) at Android.MODEL (Redmi Note 8T) at Android.VERSION (:11) at Android.FINGERPRINT
We don't have any message for this stack (thanks to the Play Store...) and I can't figure why it's happening. The nullabily is correctly checked and
media
is marked as not null. To improve stability, I willtry catch
this, but if you have some insight, it would be useful.Hi @Aza,
Yeah, that's really strange. If you go into
isPathValid
, the 1st line is a new File(String) which throws an NPE in the constructor. I suppose that's the source of the exception, the question is how we arrive in the constructor with a null argument in the first place. It would seem to me the isNullOrEmpty() call should be performing that check. Is it possible for the media.artworkMrl field to be modified on a different thread or coroutine concurrently? If so, that's not a simple fix, so we have to take a defensive approach to solve this particular issue and then go hunting for the race condition later.What about dropping the
isNullOrEmpty
inupdateMediaQueue
and moving that logic into isPathValid?You could do this:
fun isPathValid(path: String?): Boolean { if (path.isNullOrEmpty()) return false... }
before calling
val file = File(path)
, which we know will blow up. This has the added benefit of making isPathValid more robust since we're evaluating the string directly, and not the field in the MediaWrapper object (which updateMediaQueue does not fully own/control) at two separate times.Edited by Robert StoneIf you go into isPathValid, the 1st line is a new File(String) which throws an NPE in the constructor. I suppose that's the source of the exception
I don't think so. if it was the case, we would have a line in
File.java
at the top of the stack trace.I also was thinking about a race condition between
media.artworkMrl.isNullOrEmpty()
andisPathValid(media.artworkMrl)
but what I don't understans is that it taken for a list copy and this copy cannot not be changed in the meantime.If it's indeed a race condition between the two parts of the condition, a solution could be to extract the
artworkMrl
in a variable and use it for the nullability chack and theisPathValid
method.Your solution may work as well.
Ah, good point regarding the stack trace. I am able to trigger a similar NPE at the getOrPut, too. If you do extract the string, make sure to use it here as well.
val nullString: String? = null
val artworkUri = artworkToUriCache.getOrPut(nullString!!, { ArtworkProvider.buildMediaUri(media) } )
My strategy will be to change the whole block by:
for (media in playlistManager.getMediaList()) { try { val artworkMrl = media.artworkMrl if (!artworkMrl.isNullOrEmpty() && isPathValid(artworkMrl)) { val artworkUri = artworkToUriCache.getOrPut(artworkMrl, { ArtworkProvider.buildMediaUri(media) } ) val key = MediaSessionBrowser.generateMediaId(media) it[key] = artworkUri } } catch (e: java.lang.NullPointerException) { Log.e("PlaybackService", "Caught NullPointerException", e) VLCCrashHandler.saveLog(e) } }
VLCCrashHandler.saveLog()
is a new mechanism I am working on that will save the crash and logs when in beta to trigger the Crash Reporter. That way, we will probably have the stack trace by mail during our new beta round. to better understand what is going on.See !1098 (merged)
Hi @Aza,
I took a bit of time to perform additional debugging and re-read the stack trace. I am curious if
playlistManager
is null. Although I had initially suspectedisPathValid
, Kotlin insertskotlin.jvm.internal.Intrinsics
calls to verify parameters are not-null. If an NPE is generated, a message is attached with the problematic field name. In this case there is no message, so it may be something else...If we look back at line 1182, which is 1094, we see:
for (media in playlistManager.getMediaList()) {
There really isn't much to throw an NPE except
playlistManager
itself being null. If that is the case, the try-catch block as-is will be insufficient to capture the problem. I took your mouse-trap and enlarged it to trap a bigger critter. The code doesn't add null checks to stop the NPE from occurring--we want to see it happening--but it will capture the problem.In the method below I isolated each call onto a separate line. This should yield a stack trace pinpointing the null value. The log message was also updated to identify if either variable is null.
private fun updateMediaQueue() = lifecycleScope.launch(start = CoroutineStart.UNDISPATCHED) { if (!this@PlaybackService::mediaSession.isInitialized) initMediaSession() artworkMap = HashMap<String, Uri>().also { val artworkToUriCache = HashMap<String, Uri>() var mqPlaylistManager: PlaylistManager? = null var mqMediaList: List<MediaWrapper>? = null try { mqPlaylistManager = playlistManager mqMediaList = mqPlaylistManager.getMediaList() mqMediaList.forEach { media -> val artworkMrl = media.artworkMrl if (!artworkMrl.isNullOrEmpty() && isPathValid(artworkMrl)) { val artworkUri = artworkToUriCache.getOrPut(artworkMrl, { ArtworkProvider.buildMediaUri(media) } ) val key = MediaSessionBrowser.generateMediaId(media) it[key] = artworkUri } } } catch (e: java.lang.NullPointerException) { Log.e(TAG, "Caught NullPointerException. PlaylistManager: ${mqPlaylistManager != null} MediaList: ${mqMediaList != null}", e) VLCCrashHandler.saveLog(e) } artworkToUriCache.clear() } updateMediaQueueSlidingWindow(true) }
As I traced the call to
playlistManager
, I noticed the declaration aslateinit var playlistManager: PlaylistManager
. Is it possibleupdateMediaQueue
is invoked prior toonCreate()
whereplaylistManager
is initialized?I am really sorry, I've been misleading. The line 1182 of the
3.4.0
tag is not the 1094 one here but the 1095.Anyway,
playlistManager
cannot be null (as it's declared as not null) but can be uninitialized and therefore it would throw aUninitializedPropertyAccessException
.