Skip to content

Commit

Permalink
Fix player menu track selection
Browse files Browse the repository at this point in the history
Because the menus use setGroupCheckable with exclusive=true, it isn't necessary to uncheck the other items.
Furthermore, while transcoding, the media source is updated and the menu rebuilt (with the new selections already). This causes the clickedItem to become outdated/invalid in the fragment.onXTrackSelected callback. As a result, the selection will be invalidated.
To fix this, we assume that selections are always successful and immediately apply the selection to the clicked item before notifying the fragment.
  • Loading branch information
Maxr1998 authored and nielsvanvelzen committed May 13, 2023
1 parent f7202ec commit cf52144
Showing 1 changed file with 14 additions and 23 deletions.
37 changes: 14 additions & 23 deletions app/src/main/java/org/jellyfin/mobile/player/ui/PlayerMenus.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import android.widget.ImageButton
import android.widget.PopupMenu
import android.widget.TextView
import androidx.annotation.StringRes
import androidx.core.view.forEach
import androidx.core.view.get
import androidx.core.view.isVisible
import androidx.core.view.size
Expand Down Expand Up @@ -181,13 +180,14 @@ class PlayerMenus(

private fun createSubtitlesMenu() = PopupMenu(context, subtitlesButton).apply {
setOnMenuItemClickListener { clickedItem ->
val selected = clickedItem.itemId
fragment.onSubtitleSelected(selected) {
menu.forEach { item ->
item.isChecked = false
}
clickedItem.isChecked = true
subtitlesEnabled = selected >= 0
// Immediately apply changes to the menu, necessary when direct playing
// When transcoding, the updated media source will cause the menu to be rebuilt
clickedItem.isChecked = true

// The itemId is the MediaStream.index of the track
val selectedSubtitleStreamIndex = clickedItem.itemId
fragment.onSubtitleSelected(selectedSubtitleStreamIndex) {
subtitlesEnabled = selectedSubtitleStreamIndex >= 0
updateSubtitlesButton()
}
true
Expand All @@ -197,13 +197,12 @@ class PlayerMenus(

private fun createAudioStreamsMenu() = PopupMenu(context, audioStreamsButton).apply {
setOnMenuItemClickListener { clickedItem: MenuItem ->
// Immediately apply changes to the menu, necessary when direct playing
// When transcoding, the updated media source will cause the menu to be rebuilt
clickedItem.isChecked = true

// The itemId is the MediaStream.index of the track
fragment.onAudioTrackSelected(clickedItem.itemId) {
menu.forEach { item ->
item.isChecked = false
}
clickedItem.isChecked = true
}
fragment.onAudioTrackSelected(clickedItem.itemId) {}
true
}
setOnDismissListener(this@PlayerMenus)
Expand All @@ -217,12 +216,7 @@ class PlayerMenus(
menu.setGroupCheckable(SPEED_MENU_GROUP, true, true)
setOnMenuItemClickListener { clickedItem: MenuItem ->
fragment.onSpeedSelected(clickedItem.itemId * SPEED_MENU_STEP_SIZE).also { success ->
if (success) {
menu.forEach { item ->
item.isChecked = false
}
clickedItem.isChecked = true
}
if (success) clickedItem.isChecked = true
}
}
setOnDismissListener(this@PlayerMenus)
Expand Down Expand Up @@ -257,9 +251,6 @@ class PlayerMenus(
setOnMenuItemClickListener { clickedItem: MenuItem ->
val type = DecoderType.values()[clickedItem.itemId]
fragment.onDecoderSelected(type)
menu.forEach { item ->
item.isChecked = false
}
clickedItem.isChecked = true
true
}
Expand Down

0 comments on commit cf52144

Please sign in to comment.