Skip to content
Snippets Groups Projects

Add sliding window for the playback queue when in Android Auto mode.

Merged Robert Stone requested to merge rhstone/vlc-android:add-sliding-window-for-queue into master
1 unresolved thread

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

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Nicolas Pomepuy
  • Robert Stone added 1 commit

    added 1 commit

    • a5fbff12 - Add sliding window for the playback queue when in Android Auto mode. Threading...

    Compare with previous version

  • Robert Stone added 2 commits

    added 2 commits

    • 85677afc - 1 commit from branch videolan:master
    • d713f1a8 - Add sliding window for the playback queue when in Android Auto mode. Threading...

    Compare with previous version

  • Robert Stone added 1 commit

    added 1 commit

    • 7438d76d - Add sliding window for the playback queue when in Android Auto mode. Threading...

    Compare with previous version

  • Robert Stone added 1 commit

    added 1 commit

    • 9be14388 - Add sliding window for the playback queue when in Android Auto mode.

    Compare with previous version

  • Robert Stone
  • Nicolas Pomepuy resolved all threads

    resolved all threads

  • Nicolas Pomepuy added 3 commits

    added 3 commits

    • 9be14388...8c991851 - 2 commits from branch videolan:master
    • 603b9386 - Add sliding window for the playback queue when in Android Auto mode.

    Compare with previous version

  • Nicolas Pomepuy enabled an automatic merge when the pipeline for 603b9386 succeeds

    enabled an automatic merge when the pipeline for 603b9386 succeeds

  • 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 will try catch this, but if you have some insight, it would be useful.

    • Author Developer

      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 in updateMediaQueue 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 Stone
    • Author Developer

      Note: Updated isPathValid signature to accept a null String argument, otherwise the Kotlin runtime will throw an NPE.

    • 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

      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() and isPathValid(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 the isPathValid method.

      Your solution may work as well.

    • Author Developer

      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.

    • Author Developer

      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 suspected isPathValid, Kotlin inserts kotlin.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 as lateinit var playlistManager: PlaylistManager. Is it possible updateMediaQueue is invoked prior to onCreate() where playlistManager 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.

      See https://code.videolan.org/videolan/vlc-android/-/blob/a935b161c28f0dc12121746f656e9edb7d7b5d6d/application/vlc-android/src/org/videolan/vlc/PlaybackService.kt#L1182

      Anyway, playlistManager cannot be null (as it's declared as not null) but can be uninitialized and therefore it would throw a UninitializedPropertyAccessException.

    • Please register or sign in to reply
    Please register or sign in to reply
    Loading