Skip to content

Commit

Permalink
playback: implement "safe" slider wrapper
Browse files Browse the repository at this point in the history
Implement a safe slider wrapper that does not crash with invalid values
as often.

Slider is a terrible component that is not designed with Auxio's
use-case in the slightest. Instead of gracefully degrading with invalid
values, it just crashes the entire app, which is horrible for UX.

Since SeekBar is a useless buggy version-specific sh******ed mess too,
we have no choice but to wrap Slider in a safe view layout that
hopefully hacks with the input enough to not crash the app when doing
simple seeking actions.

I hate android so much.

Resolves #140.
  • Loading branch information
OxygenCobalt committed May 27, 2022
1 parent 852630a commit c6d7d8f
Show file tree
Hide file tree
Showing 12 changed files with 201 additions and 211 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## dev [v2.3.1, v2.4.0, or v3.0.0]

#### What's Fixed
- Fixed crash when seeking to the end of a track as the track changed to a track with a lower duration

## v2.3.0

#### What's New
Expand Down
1 change: 1 addition & 0 deletions app/src/main/java/org/oxycblt/auxio/music/Music.kt
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ data class Song(
val uri: Uri
get() =
ContentUris.withAppendedId(MediaStore.Audio.Media.EXTERNAL_CONTENT_URI, _mediaStoreId)

/** The duration of this song, in seconds (rounded down) */
val durationSecs: Long
get() = durationMs / 1000
Expand Down
4 changes: 4 additions & 0 deletions app/src/main/java/org/oxycblt/auxio/music/MusicViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ class MusicViewModel : ViewModel(), MusicStore.Callback {
}
}

/**
* Reload the music library. Note that this call will result in unexpected behavior in the case
* that music is reloaded after a loading process has already exceeded.
*/
fun reloadMusic(context: Context) {
logD("Reloading music library")
_loaderResponse.value = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ import androidx.appcompat.widget.Toolbar
import androidx.core.view.updatePadding
import androidx.fragment.app.Fragment
import androidx.fragment.app.activityViewModels
import com.google.android.material.slider.Slider
import kotlin.math.max
import org.oxycblt.auxio.R
import org.oxycblt.auxio.databinding.FragmentPlaybackPanelBinding
import org.oxycblt.auxio.music.MusicParent
Expand All @@ -34,7 +32,6 @@ import org.oxycblt.auxio.playback.state.RepeatMode
import org.oxycblt.auxio.ui.MainNavigationAction
import org.oxycblt.auxio.ui.NavigationViewModel
import org.oxycblt.auxio.ui.ViewBindingFragment
import org.oxycblt.auxio.util.formatDuration
import org.oxycblt.auxio.util.logD
import org.oxycblt.auxio.util.systemBarInsetsCompat
import org.oxycblt.auxio.util.textSafe
Expand All @@ -50,8 +47,7 @@ import org.oxycblt.auxio.util.textSafe
*/
class PlaybackPanelFragment :
ViewBindingFragment<FragmentPlaybackPanelBinding>(),
Slider.OnChangeListener,
Slider.OnSliderTouchListener,
StyledSeekBar.Callback,
Toolbar.OnMenuItemClickListener {
private val playbackModel: PlaybackViewModel by activityViewModels()
private val navModel: NavigationViewModel by activityViewModels()
Expand Down Expand Up @@ -93,10 +89,7 @@ class PlaybackPanelFragment :
playbackModel.song.value?.let { navModel.exploreNavigateTo(it.album) }
}

binding.playbackSeekBar.apply {
addOnChangeListener(this@PlaybackPanelFragment)
addOnSliderTouchListener(this@PlaybackPanelFragment)
}
binding.playbackSeekBar.callback = this

binding.playbackRepeat.setOnClickListener { playbackModel.incrementRepeatMode() }
binding.playbackSkipPrev.setOnClickListener { playbackModel.prev() }
Expand All @@ -110,8 +103,6 @@ class PlaybackPanelFragment :
binding.playbackSkipNext.setOnClickListener { playbackModel.next() }
binding.playbackShuffle.setOnClickListener { playbackModel.invertShuffled() }

binding.playbackSeekBar.apply {}

// --- VIEWMODEL SETUP --

playbackModel.song.observe(viewLifecycleOwner, ::updateSong)
Expand All @@ -133,8 +124,7 @@ class PlaybackPanelFragment :
override fun onDestroyBinding(binding: FragmentPlaybackPanelBinding) {
binding.playbackToolbar.setOnMenuItemClickListener(null)
binding.playbackSong.isSelected = false
binding.playbackSeekBar.removeOnChangeListener(this)
binding.playbackSeekBar.removeOnChangeListener(this)
binding.playbackSeekBar.callback = null
}

override fun onMenuItemClick(item: MenuItem): Boolean {
Expand All @@ -147,19 +137,8 @@ class PlaybackPanelFragment :
}
}

override fun onStartTrackingTouch(slider: Slider) {
requireBinding().playbackPosition.isActivated = true
}

override fun onStopTrackingTouch(slider: Slider) {
requireBinding().playbackPosition.isActivated = false
playbackModel.seekTo(slider.value.toLong())
}

override fun onValueChange(slider: Slider, value: Float, fromUser: Boolean) {
if (fromUser) {
requireBinding().playbackPosition.textSafe = value.toLong().formatDuration(true)
}
override fun seekTo(positionSecs: Long) {
playbackModel.seekTo(positionSecs)
}

private fun updateSong(song: Song?) {
Expand All @@ -171,29 +150,16 @@ class PlaybackPanelFragment :
binding.playbackSong.textSafe = song.resolveName(context)
binding.playbackArtist.textSafe = song.resolveIndividualArtistName(context)
binding.playbackAlbum.textSafe = song.album.resolveName(context)

// Normally if a song had a duration
val seconds = song.durationSecs
binding.playbackDuration.textSafe = seconds.formatDuration(false)
binding.playbackSeekBar.apply {
isEnabled = seconds > 0L
valueTo = max(seconds, 1L).toFloat()
}
binding.playbackSeekBar.durationSecs = song.durationSecs
}

private fun updateParent(parent: MusicParent?) {
requireBinding().playbackToolbar.subtitle =
parent?.resolveName(requireContext()) ?: getString(R.string.lbl_all_songs)
}

private fun updatePosition(position: Long) {
// Don't update the progress while we are seeking, that will make the SeekBar jump
// around.
val binding = requireBinding()
if (!binding.playbackPosition.isActivated) {
binding.playbackSeekBar.value = position.toFloat()
binding.playbackPosition.textSafe = position.formatDuration(true)
}
private fun updatePosition(positionSecs: Long) {
requireBinding().playbackSeekBar.positionSecs = positionSecs
}

private fun updateRepeat(repeatMode: RepeatMode) {
Expand Down
125 changes: 125 additions & 0 deletions app/src/main/java/org/oxycblt/auxio/playback/StyledSeekBar.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
/*
* Copyright (c) 2022 Auxio Project
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/

package org.oxycblt.auxio.playback

import android.content.Context
import android.util.AttributeSet
import android.widget.FrameLayout
import com.google.android.material.slider.Slider
import kotlin.math.max
import org.oxycblt.auxio.databinding.ViewSeekBarBinding
import org.oxycblt.auxio.util.formatDuration
import org.oxycblt.auxio.util.inflater
import org.oxycblt.auxio.util.logD
import org.oxycblt.auxio.util.textSafe

/**
* A wrapper around [Slider] that shows not only position and duration values, but also basically
* hacks in behavior consistent with a normal SeekBar in a way that will not crash the app.
*
* SeekBar, like most android OS components, is a version-specific mess that requires constant
* hacks on older versions. Instead, we use the more "modern" slider component, but it is not
* designed for the job that Auxio's progress bar has. It does not gracefully degrade when
* positions don't make sense (which happens incredibly often), it just crashes the entire app,
* which is insane but also checks out for something more meant for configuration than seeking.
*
* Instead, we wrap it in a safe class that hopefully implements enough safety to not crash the
* app or result in blatantly janky behavior. Mostly.
*
* @author OxygenCobalt
*/
class StyledSeekBar
@JvmOverloads
constructor(
context: Context,
attrs: AttributeSet? = null,
defStyleAttr: Int = 0,
) :
FrameLayout(context, attrs, defStyleAttr),
Slider.OnSliderTouchListener,
Slider.OnChangeListener {
private val binding = ViewSeekBarBinding.inflate(context.inflater, this, true)

init {
binding.seekBarSlider.addOnSliderTouchListener(this)
binding.seekBarSlider.addOnChangeListener(this)
}

var callback: Callback? = null

/**
* The current position, in seconds. This is the current value of the SeekBar and is indicated
* by the start TextView in the layout.
*/
var positionSecs: Long
get() = binding.seekBarSlider.value.toLong()
set(value) {
// Sanity check: Ensure that this value is within the duration and will not crash
// the app, and that the user is not currently seeking (which would cause the SeekBar
// to jump around).
if (value <= durationSecs && !isActivated) {
binding.seekBarSlider.value = value.toFloat()
}
}

/**
* The current duration, in seconds. This is the end value of the SeekBar and is indicated
* by the end TextView in the layout.
*/
var durationSecs: Long
get() = binding.seekBarSlider.valueTo.toLong()
set(value) {
// Sanity check 1: If this is a value so low that it effectively rounds down to
// zero, use 1 instead and disable the SeekBar.
val to = max(value, 1)
isEnabled = value > 0

// Sanity check 2: If the current value exceeds the new duration value, clamp it
// down so that we don't crash and instead have an annoying visual flicker.
if (positionSecs > to) {
binding.seekBarSlider.value = to.toFloat()
}

binding.seekBarSlider.valueTo = to.toFloat()
binding.seekBarDuration.textSafe = value.formatDuration(false)
}

override fun onStartTrackingTouch(slider: Slider) {
// User has begun seeking, place the SeekBar into a "Suspended" mode in which no
// position updates are sent and is indicated by the position value turning accented.
isActivated = true
}

override fun onStopTrackingTouch(slider: Slider) {
// End of seek event, send off new value to callback.
isActivated = false
callback?.seekTo(slider.value.toLong())
}

override fun onValueChange(slider: Slider, value: Float, fromUser: Boolean) {
binding.seekBarPosition.textSafe = value.toLong().formatDuration(true)
}

interface Callback {
/**
* Called when a seek event was completed and the new position must be seeked to by
* the app.
*/
fun seekTo(positionSecs: Long)
}
}
36 changes: 2 additions & 34 deletions app/src/main/res/layout-land/fragment_playback_panel.xml
Original file line number Diff line number Diff line change
Expand Up @@ -77,46 +77,14 @@
app:layout_constraintTop_toBottomOf="@+id/playback_artist"
tools:text="Album Name" />

<com.google.android.material.slider.Slider
<org.oxycblt.auxio.playback.StyledSeekBar
android:id="@+id/playback_seek_bar"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginBottom="@dimen/spacing_medium"
android:valueFrom="0"
android:valueTo="1"
app:haloRadius="@dimen/slider_halo_radius"
app:labelBehavior="gone"
app:labelStyle="@style/TextAppearance.Auxio.BodySmall"
app:layout_constraintBottom_toTopOf="@+id/playback_play_pause"
app:layout_constraintEnd_toEndOf="@+id/playback_song_container"
app:layout_constraintHorizontal_bias="0.0"
app:layout_constraintStart_toStartOf="parent"
app:thumbRadius="@dimen/slider_thumb_radius"
app:trackColorInactive="@color/sel_track" />

<TextView
android:id="@+id/playback_position"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="@dimen/spacing_medium"
android:layout_marginTop="@dimen/spacing_small_inv"
android:textAppearance="@style/TextAppearance.Auxio.BodyMedium"
android:textColor="@color/sel_accented_secondary"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/playback_seek_bar"
tools:text="11:38" />

<TextView
android:id="@+id/playback_duration"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/spacing_small_inv"
android:layout_marginEnd="@dimen/spacing_medium"
android:textAppearance="@style/TextAppearance.Auxio.BodyMedium"
android:textColor="?android:attr/textColorSecondary"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toBottomOf="@+id/playback_seek_bar"
tools:text="16:16" />
app:layout_constraintStart_toStartOf="parent" />

<org.oxycblt.auxio.ui.StyledImageButton
android:id="@+id/playback_repeat"
Expand Down
36 changes: 2 additions & 34 deletions app/src/main/res/layout-sw600dp-land/fragment_playback_panel.xml
Original file line number Diff line number Diff line change
Expand Up @@ -72,49 +72,17 @@
app:layout_constraintTop_toBottomOf="@+id/playback_artist"
tools:text="Album Name" />

<com.google.android.material.slider.Slider
<org.oxycblt.auxio.playback.StyledSeekBar
android:id="@+id/playback_seek_bar"
android:layout_width="0dp"
android:layout_height="wrap_content"
android:layout_marginStart="@dimen/spacing_small"
android:layout_marginEnd="@dimen/spacing_small"
android:layout_marginBottom="@dimen/spacing_medium"
android:valueFrom="0"
android:valueTo="1"
app:haloRadius="@dimen/slider_halo_radius"
app:labelBehavior="gone"
app:labelStyle="@style/TextAppearance.Auxio.BodySmall"
app:layout_constraintBottom_toTopOf="@+id/playback_play_pause"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintHorizontal_bias="0.5"
app:layout_constraintStart_toEndOf="@+id/playback_cover"
app:layout_constraintTop_toBottomOf="@+id/playback_album"
app:thumbRadius="@dimen/slider_thumb_radius"
app:trackColorInactive="@color/sel_track" />

<TextView
android:id="@+id/playback_position"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginStart="@dimen/spacing_medium"
android:layout_marginTop="@dimen/spacing_small_inv"
android:textAppearance="@style/TextAppearance.Auxio.BodyMedium"
android:textColor="@color/sel_accented_secondary"
app:layout_constraintStart_toStartOf="@+id/playback_seek_bar"
app:layout_constraintTop_toBottomOf="@+id/playback_seek_bar"
tools:text="11:38" />

<TextView
android:id="@+id/playback_duration"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/spacing_small_inv"
android:layout_marginEnd="@dimen/spacing_medium"
android:textAppearance="@style/TextAppearance.Auxio.BodyMedium"
android:textColor="?android:attr/textColorSecondary"
app:layout_constraintEnd_toEndOf="@+id/playback_seek_bar"
app:layout_constraintTop_toBottomOf="@+id/playback_seek_bar"
tools:text="16:16" />
app:layout_constraintTop_toBottomOf="@+id/playback_album" />

<org.oxycblt.auxio.ui.StyledImageButton
android:id="@+id/playback_repeat"
Expand Down
Loading

0 comments on commit c6d7d8f

Please sign in to comment.