Reworked/Implemented PlaybackParameterDialog functionallity

* Add support for semitones
* Fixed some minor bugs
* Improved some methods

app/src/main/java/org/schabi/newpipe/player/helper/PlaybackParameterDialog.java

@@ -8,11 +8,14 @@ import android.content.Context;
 import android.os.Bundle;
 import android.util.Log;
 import android.view.LayoutInflater;
+import android.view.View;
+import android.widget.CheckBox;
 import android.widget.SeekBar;
 import android.widget.TextView;
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import androidx.annotation.StringRes;
 import androidx.appcompat.app.AlertDialog;
 import androidx.fragment.app.DialogFragment;
 import androidx.preference.PreferenceManager;
@@ -22,8 +25,10 @@ import org.schabi.newpipe.databinding.DialogPlaybackParameterBinding;
 import org.schabi.newpipe.util.SliderStrategy;
 
 import java.util.Objects;
+import java.util.function.Consumer;
 import java.util.function.DoubleConsumer;
 import java.util.function.DoubleFunction;
+import java.util.function.DoubleSupplier;
 
 import icepick.Icepick;
 import icepick.State;
@@ -32,8 +37,8 @@ public class PlaybackParameterDialog extends DialogFragment {
     private static final String TAG = "PlaybackParameterDialog";
 
     // Minimum allowable range in ExoPlayer
-    private static final double MINIMUM_PLAYBACK_VALUE = 0.10f;
+    private static final double MIN_PLAYBACK_VALUE = 0.10f;
-    private static final double MAXIMUM_PLAYBACK_VALUE = 3.00f;
+    private static final double MAX_PLAYBACK_VALUE = 3.00f;
 
     private static final double STEP_1_PERCENT_VALUE = 0.01f;
     private static final double STEP_5_PERCENT_VALUE = 0.05f;
@@ -42,30 +47,42 @@ public class PlaybackParameterDialog extends DialogFragment {
     private static final double STEP_100_PERCENT_VALUE = 1.00f;
 
     private static final double DEFAULT_TEMPO = 1.00f;
-    private static final double DEFAULT_PITCH = 1.00f;
+    private static final double DEFAULT_PITCH_PERCENT = 1.00f;
     private static final double DEFAULT_STEP = STEP_25_PERCENT_VALUE;
     private static final boolean DEFAULT_SKIP_SILENCE = false;
 
     private static final SliderStrategy QUADRATIC_STRATEGY = new SliderStrategy.Quadratic(
-            MINIMUM_PLAYBACK_VALUE,
+            MIN_PLAYBACK_VALUE,
-            MAXIMUM_PLAYBACK_VALUE,
+            MAX_PLAYBACK_VALUE,
             1.00f,
             10_000);
 
+    private static final SliderStrategy SEMITONE_STRATEGY = new SliderStrategy() {
+        @Override
+        public int progressOf(final double value) {
+            return PlayerSemitoneHelper.percentToSemitones(value) + 12;
+        }
+
+        @Override
+        public double valueOf(final int progress) {
+            return PlayerSemitoneHelper.semitonesToPercent(progress - 12);
+        }
+    };
+
     @Nullable
     private Callback callback;
 
     @State
     double initialTempo = DEFAULT_TEMPO;
     @State
-    double initialPitch = DEFAULT_PITCH;
+    double initialPitchPercent = DEFAULT_PITCH_PERCENT;
     @State
     boolean initialSkipSilence = DEFAULT_SKIP_SILENCE;
 
     @State
     double tempo = DEFAULT_TEMPO;
     @State
-    double pitch = DEFAULT_PITCH;
+    double pitchPercent = DEFAULT_PITCH_PERCENT;
     @State
     double stepSize = DEFAULT_STEP;
     @State
@@ -83,11 +100,11 @@ public class PlaybackParameterDialog extends DialogFragment {
         dialog.callback = callback;
 
         dialog.initialTempo = playbackTempo;
-        dialog.initialPitch = playbackPitch;
+        dialog.initialPitchPercent = playbackPitch;
         dialog.initialSkipSilence = playbackSkipSilence;
 
         dialog.tempo = dialog.initialTempo;
-        dialog.pitch = dialog.initialPitch;
+        dialog.pitchPercent = dialog.initialPitchPercent;
         dialog.skipSilence = dialog.initialSkipSilence;
 
         return dialog;
@@ -125,20 +142,19 @@ public class PlaybackParameterDialog extends DialogFragment {
 
         binding = DialogPlaybackParameterBinding.inflate(LayoutInflater.from(getContext()));
         initUI();
-        initUIData();
 
         final AlertDialog.Builder dialogBuilder = new AlertDialog.Builder(requireActivity())
                 .setView(binding.getRoot())
                 .setCancelable(true)
                 .setNegativeButton(R.string.cancel, (dialogInterface, i) -> {
                     setAndUpdateTempo(initialTempo);
-                    setAndUpdatePitch(initialPitch);
+                    setAndUpdatePitch(initialPitchPercent);
                     setAndUpdateSkipSilence(initialSkipSilence);
                     updateCallback();
                 })
                 .setNeutralButton(R.string.playback_reset, (dialogInterface, i) -> {
                     setAndUpdateTempo(DEFAULT_TEMPO);
-                    setAndUpdatePitch(DEFAULT_PITCH);
+                    setAndUpdatePitch(DEFAULT_PITCH_PERCENT);
                     setAndUpdateSkipSilence(DEFAULT_SKIP_SILENCE);
                     updateCallback();
                 })
@@ -153,12 +169,63 @@ public class PlaybackParameterDialog extends DialogFragment {
 
     private void initUI() {
         // Tempo
-        setText(binding.tempoMinimumText, PlayerHelper::formatSpeed, MINIMUM_PLAYBACK_VALUE);
+        setText(binding.tempoMinimumText, PlayerHelper::formatSpeed, MIN_PLAYBACK_VALUE);
-        setText(binding.tempoMaximumText, PlayerHelper::formatSpeed, MAXIMUM_PLAYBACK_VALUE);
+        setText(binding.tempoMaximumText, PlayerHelper::formatSpeed, MAX_PLAYBACK_VALUE);
 
-        // Pitch
+        binding.tempoSeekbar.setMax(QUADRATIC_STRATEGY.progressOf(MAX_PLAYBACK_VALUE));
-        setText(binding.pitchMinimumText, PlayerHelper::formatPitch, MINIMUM_PLAYBACK_VALUE);
+        setAndUpdateTempo(tempo);
-        setText(binding.pitchMaximumText, PlayerHelper::formatPitch, MAXIMUM_PLAYBACK_VALUE);
+        binding.tempoSeekbar.setOnSeekBarChangeListener(
+                getTempoOrPitchSeekbarChangeListener(
+                        QUADRATIC_STRATEGY,
+                        this::onTempoSliderUpdated));
+
+        registerOnStepClickListener(
+                binding.tempoStepDown,
+                () -> tempo,
+                -1,
+                this::onTempoSliderUpdated);
+        registerOnStepClickListener(
+                binding.tempoStepUp,
+                () -> tempo,
+                1,
+                this::onTempoSliderUpdated);
+
+        // Pitch - Percent
+        setText(binding.pitchPercentMinimumText, PlayerHelper::formatPitch, MIN_PLAYBACK_VALUE);
+        setText(binding.pitchPercentMaximumText, PlayerHelper::formatPitch, MAX_PLAYBACK_VALUE);
+
+        binding.pitchPercentSeekbar.setMax(QUADRATIC_STRATEGY.progressOf(MAX_PLAYBACK_VALUE));
+        setAndUpdatePitch(pitchPercent);
+        binding.pitchPercentSeekbar.setOnSeekBarChangeListener(
+                getTempoOrPitchSeekbarChangeListener(
+                        QUADRATIC_STRATEGY,
+                        this::onPitchPercentSliderUpdated));
+
+        registerOnStepClickListener(
+                binding.pitchPercentStepDown,
+                () -> pitchPercent,
+                -1,
+                this::onPitchPercentSliderUpdated);
+        registerOnStepClickListener(
+                binding.pitchPercentStepUp,
+                () -> pitchPercent,
+                1,
+                this::onPitchPercentSliderUpdated);
+
+        // Pitch - Semitone
+        binding.pitchSemitoneSeekbar.setOnSeekBarChangeListener(
+                getTempoOrPitchSeekbarChangeListener(
+                        SEMITONE_STRATEGY,
+                        this::onPitchPercentSliderUpdated));
+
+        registerOnSemitoneStepClickListener(
+                binding.pitchSemitoneStepDown,
+                -1,
+                this::onPitchPercentSliderUpdated);
+        registerOnSemitoneStepClickListener(
+                binding.pitchSemitoneStepUp,
+                1,
+                this::onPitchPercentSliderUpdated);
 
         // Steps
         setupStepTextView(binding.stepSizeOnePercent, STEP_1_PERCENT_VALUE);
@@ -166,6 +233,34 @@ public class PlaybackParameterDialog extends DialogFragment {
         setupStepTextView(binding.stepSizeTenPercent, STEP_10_PERCENT_VALUE);
         setupStepTextView(binding.stepSizeTwentyFivePercent, STEP_25_PERCENT_VALUE);
         setupStepTextView(binding.stepSizeOneHundredPercent, STEP_100_PERCENT_VALUE);
+
+        setAndUpdateStepSize(stepSize);
+
+        // Bottom controls
+        bindCheckboxWithBoolPref(
+                binding.unhookCheckbox,
+                R.string.playback_unhook_key,
+                true,
+                isChecked -> {
+                    if (!isChecked) {
+                        // when unchecked, slide back to the minimum of current tempo or pitch
+                        setSliders(Math.min(pitchPercent, tempo));
+                        updateCallback();
+                    }
+                });
+
+        setAndUpdateSkipSilence(skipSilence);
+        binding.skipSilenceCheckbox.setOnCheckedChangeListener((compoundButton, isChecked) -> {
+            skipSilence = isChecked;
+            updateCallback();
+        });
+
+        bindCheckboxWithBoolPref(
+                binding.adjustBySemitonesCheckbox,
+                R.string.playback_adjust_by_semitones_key,
+                false,
+                this::showPitchSemitonesOrPercent
+        );
     }
 
     private TextView setText(
@@ -177,6 +272,31 @@ public class PlaybackParameterDialog extends DialogFragment {
         return textView;
     }
 
+    private void registerOnStepClickListener(
+            final TextView stepTextView,
+            final DoubleSupplier currentValueSupplier,
+            final double direction, // -1 for step down, +1 for step up
+            final DoubleConsumer newValueConsumer
+    ) {
+        stepTextView.setOnClickListener(view -> {
+            newValueConsumer.accept(
+                    currentValueSupplier.getAsDouble() + 1 * stepSize * direction);
+            updateCallback();
+        });
+    }
+
+    private void registerOnSemitoneStepClickListener(
+            final TextView stepTextView,
+            final int direction, // -1 for step down, +1 for step up
+            final DoubleConsumer newValueConsumer
+    ) {
+        stepTextView.setOnClickListener(view -> {
+            newValueConsumer.accept(PlayerSemitoneHelper.semitonesToPercent(
+                    PlayerSemitoneHelper.percentToSemitones(this.pitchPercent) + direction));
+            updateCallback();
+        });
+    }
+
     private void setupStepTextView(
             final TextView textView,
             final double stepSizeValue
@@ -185,77 +305,14 @@ public class PlaybackParameterDialog extends DialogFragment {
                 .setOnClickListener(view -> setAndUpdateStepSize(stepSizeValue));
     }
 
-    private void initUIData() {
-        // Tempo
-        binding.tempoSeekbar.setMax(QUADRATIC_STRATEGY.progressOf(MAXIMUM_PLAYBACK_VALUE));
-        setAndUpdateTempo(tempo);
-        binding.tempoSeekbar.setOnSeekBarChangeListener(
-                getTempoOrPitchSeekbarChangeListener(this::onTempoSliderUpdated));
-
-        registerOnStepClickListener(
-                binding.tempoStepDown, tempo, -1, this::onTempoSliderUpdated);
-        registerOnStepClickListener(
-                binding.tempoStepUp, tempo, 1, this::onTempoSliderUpdated);
-
-        // Pitch
-        binding.pitchSeekbar.setMax(QUADRATIC_STRATEGY.progressOf(MAXIMUM_PLAYBACK_VALUE));
-        setAndUpdatePitch(pitch);
-        binding.pitchSeekbar.setOnSeekBarChangeListener(
-                getTempoOrPitchSeekbarChangeListener(this::onPitchSliderUpdated));
-
-        registerOnStepClickListener(
-                binding.pitchStepDown, pitch, -1, this::onPitchSliderUpdated);
-        registerOnStepClickListener(
-                binding.pitchStepUp, pitch, 1, this::onPitchSliderUpdated);
-
-        // Steps
-        setAndUpdateStepSize(stepSize);
-
-        // Bottom controls
-        // restore whether pitch and tempo are unhooked or not
-        binding.unhookCheckbox.setChecked(PreferenceManager
-                .getDefaultSharedPreferences(requireContext())
-                .getBoolean(getString(R.string.playback_unhook_key), true));
-
-        binding.unhookCheckbox.setOnCheckedChangeListener((compoundButton, isChecked) -> {
-            // save whether pitch and tempo are unhooked or not
-            PreferenceManager.getDefaultSharedPreferences(requireContext())
-                    .edit()
-                    .putBoolean(getString(R.string.playback_unhook_key), isChecked)
-                    .apply();
-
-            if (!isChecked) {
-                // when unchecked, slide back to the minimum of current tempo or pitch
-                setSliders(Math.min(pitch, tempo));
-            }
-        });
-
-        setAndUpdateSkipSilence(skipSilence);
-        binding.skipSilenceCheckbox.setOnCheckedChangeListener((compoundButton, isChecked) -> {
-            skipSilence = isChecked;
-            updateCallback();
-        });
-    }
-
-    private void registerOnStepClickListener(
-            final TextView stepTextView,
-            final double currentValue,
-            final double direction, // -1 for step down, +1 for step up
-            final DoubleConsumer newValueConsumer
-    ) {
-        stepTextView.setOnClickListener(view ->
-                newValueConsumer.accept(currentValue * direction)
-        );
-    }
-
     private void setAndUpdateStepSize(final double newStepSize) {
         this.stepSize = newStepSize;
 
         binding.tempoStepUp.setText(getStepUpPercentString(newStepSize));
         binding.tempoStepDown.setText(getStepDownPercentString(newStepSize));
 
-        binding.pitchStepUp.setText(getStepUpPercentString(newStepSize));
+        binding.pitchPercentStepUp.setText(getStepUpPercentString(newStepSize));
-        binding.pitchStepDown.setText(getStepDownPercentString(newStepSize));
+        binding.pitchPercentStepDown.setText(getStepDownPercentString(newStepSize));
     }
 
     private void setAndUpdateSkipSilence(final boolean newSkipSilence) {
@@ -263,19 +320,72 @@ public class PlaybackParameterDialog extends DialogFragment {
         binding.skipSilenceCheckbox.setChecked(newSkipSilence);
     }
 
+    private void bindCheckboxWithBoolPref(
+            @NonNull final CheckBox checkBox,
+            @StringRes final int resId,
+            final boolean defaultValue,
+            @Nullable final Consumer<Boolean> onInitialValueOrValueChange
+    ) {
+        final boolean prefValue = PreferenceManager
+                .getDefaultSharedPreferences(requireContext())
+                .getBoolean(getString(resId), defaultValue);
+
+        checkBox.setChecked(prefValue);
+
+        if (onInitialValueOrValueChange != null) {
+            onInitialValueOrValueChange.accept(prefValue);
+        }
+
+        checkBox.setOnCheckedChangeListener((compoundButton, isChecked) -> {
+            // save whether pitch and tempo are unhooked or not
+            PreferenceManager.getDefaultSharedPreferences(requireContext())
+                    .edit()
+                    .putBoolean(getString(resId), isChecked)
+                    .apply();
+
+            if (onInitialValueOrValueChange != null) {
+                onInitialValueOrValueChange.accept(isChecked);
+            }
+        });
+    }
+
+    private void showPitchSemitonesOrPercent(final boolean semitones) {
+        binding.pitchPercentControl.setVisibility(semitones ? View.GONE : View.VISIBLE);
+        binding.pitchSemitoneControl.setVisibility(semitones ? View.VISIBLE : View.GONE);
+
+        if (semitones) {
+            // Recalculate pitch percent when changing to semitone
+            // (as it could be an invalid semitone value)
+            final double newPitchPercent = calcValidPitch(pitchPercent);
+
+            // If the values differ set the new pitch
+            if (this.pitchPercent != newPitchPercent) {
+                if (DEBUG) {
+                    Log.d(TAG, "Bringing pitchPercent to correct corresponding semitone: "
+                            + "currentPitchPercent = " + pitchPercent + ", "
+                            + "newPitchPercent = " + newPitchPercent
+                    );
+                }
+                this.onPitchPercentSliderUpdated(newPitchPercent);
+                updateCallback();
+            }
+        }
+    }
+
     /*//////////////////////////////////////////////////////////////////////////
     // Sliders
     //////////////////////////////////////////////////////////////////////////*/
 
     private SeekBar.OnSeekBarChangeListener getTempoOrPitchSeekbarChangeListener(
+            final SliderStrategy sliderStrategy,
             final DoubleConsumer newValueConsumer
     ) {
         return new SeekBar.OnSeekBarChangeListener() {
             @Override
             public void onProgressChanged(final SeekBar seekBar, final int progress,
                                           final boolean fromUser) {
-                if (fromUser) { // this change is first in chain
+                if (fromUser) { // ensure that the user triggered the change
-                    newValueConsumer.accept(QUADRATIC_STRATEGY.valueOf(progress));
+                    newValueConsumer.accept(sliderStrategy.valueOf(progress));
                     updateCallback();
                 }
             }
@@ -300,7 +410,7 @@ public class PlaybackParameterDialog extends DialogFragment {
         }
     }
 
-    private void onPitchSliderUpdated(final double newPitch) {
+    private void onPitchPercentSliderUpdated(final double newPitch) {
         if (!binding.unhookCheckbox.isChecked()) {
             setSliders(newPitch);
         } else {
@@ -314,15 +424,39 @@ public class PlaybackParameterDialog extends DialogFragment {
     }
 
     private void setAndUpdateTempo(final double newTempo) {
-        this.tempo = newTempo;
+        this.tempo = calcValidTempo(newTempo);
+
         binding.tempoSeekbar.setProgress(QUADRATIC_STRATEGY.progressOf(tempo));
         setText(binding.tempoCurrentText, PlayerHelper::formatSpeed, tempo);
     }
 
     private void setAndUpdatePitch(final double newPitch) {
-        this.pitch = newPitch;
+        this.pitchPercent = calcValidPitch(newPitch);
-        binding.pitchSeekbar.setProgress(QUADRATIC_STRATEGY.progressOf(pitch));
+
-        setText(binding.pitchCurrentText, PlayerHelper::formatPitch, pitch);
+        binding.pitchPercentSeekbar.setProgress(QUADRATIC_STRATEGY.progressOf(pitchPercent));
+        binding.pitchSemitoneSeekbar.setProgress(SEMITONE_STRATEGY.progressOf(pitchPercent));
+        setText(binding.pitchPercentCurrentText,
+                PlayerHelper::formatPitch,
+                pitchPercent);
+        setText(binding.pitchSemitoneCurrentText,
+                PlayerSemitoneHelper::formatPitchSemitones,
+                pitchPercent);
+    }
+
+    private double calcValidTempo(final double newTempo) {
+        return Math.max(MIN_PLAYBACK_VALUE, Math.min(MAX_PLAYBACK_VALUE, newTempo));
+    }
+
+    private double calcValidPitch(final double newPitch) {
+        final double calcPitch =
+                Math.max(MIN_PLAYBACK_VALUE, Math.min(MAX_PLAYBACK_VALUE, newPitch));
+
+        if (!binding.adjustBySemitonesCheckbox.isChecked()) {
+            return calcPitch;
+        }
+
+        return PlayerSemitoneHelper.semitonesToPercent(
+                PlayerSemitoneHelper.percentToSemitones(calcPitch));
     }
 
     /*//////////////////////////////////////////////////////////////////////////
@@ -335,12 +469,12 @@ public class PlaybackParameterDialog extends DialogFragment {
         }
         if (DEBUG) {
             Log.d(TAG, "Updating callback: "
-                    + "tempo = [" + tempo + "], "
+                    + "tempo = " + tempo + ", "
-                    + "pitch = [" + pitch + "], "
+                    + "pitchPercent = " + pitchPercent + ", "
-                    + "skipSilence = [" + skipSilence + "]"
+                    + "skipSilence = " + skipSilence
             );
         }
-        callback.onPlaybackParameterChanged((float) tempo, (float) pitch, skipSilence);
+        callback.onPlaybackParameterChanged((float) tempo, (float) pitchPercent, skipSilence);
     }
 
     @NonNull

app/src/main/java/org/schabi/newpipe/player/helper/PlayerSemitoneHelper.java

@@ -0,0 +1,37 @@
+package org.schabi.newpipe.player.helper;
+
+/**
+ * Converts between percent and 12-tone equal temperament semitones.
+ * <br/>
+ * @see
+ * <a href="https://en.wikipedia.org/wiki/Equal_temperament#Twelve-tone_equal_temperament">
+ *     Wikipedia: Equal temperament#Twelve-tone equal temperament
+ * </a>
+ */
+public final class PlayerSemitoneHelper {
+    public static final int TONES = 12;
+
+    private PlayerSemitoneHelper() {
+        // No impl
+    }
+
+    public static String formatPitchSemitones(final double percent) {
+        return formatPitchSemitones(percentToSemitones(percent));
+    }
+
+    public static String formatPitchSemitones(final int semitones) {
+        return semitones > 0 ? "+" + semitones : "" + semitones;
+    }
+
+    public static double semitonesToPercent(final int semitones) {
+        return Math.pow(2, ensureSemitonesInRange(semitones) / (double) TONES);
+    }
+
+    public static int percentToSemitones(final double percent) {
+        return ensureSemitonesInRange((int) Math.round(TONES * Math.log(percent) / Math.log(2)));
+    }
+
+    private static int ensureSemitonesInRange(final int semitones) {
+        return Math.max(-TONES, Math.min(TONES, semitones));
+    }
+}