Skip to content

Commit

Permalink
Fix various nits / warnings from CI
Browse files Browse the repository at this point in the history
This includes:
- Using magic values in our enum
- Impliclt Locale issues
- Nullability checks
  • Loading branch information
DavidFair committed Jan 2, 2022
1 parent 2261ff3 commit a25c7e2
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 43 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import android.view.Display;
import android.view.WindowManager;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import org.jellyfin.androidtv.R;
Expand Down Expand Up @@ -151,11 +152,11 @@ public PlayMethod getPlaybackMethod() {
return mPlaybackMethod;
}

public void setPlaybackMethod(PlayMethod value) {
public void setPlaybackMethod(@NonNull PlayMethod value) {
mPlaybackMethod = value;
}

public void setPlaybackSpeed(Double speed){
public void setPlaybackSpeed(@NonNull Double speed) {
mRequestedPlaybackSpeed = speed;
if (hasInitializedVideoManager()) {
mVideoManager.setPlaybackSpeed(speed);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import android.view.ViewGroup;
import android.widget.FrameLayout;

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;

import com.google.android.exoplayer2.DefaultRenderersFactory;
Expand Down Expand Up @@ -143,8 +144,13 @@ public void setNativeMode(boolean value) {
}
}

public boolean isNativeMode() { return nativeMode; }
public int getZoomMode() { return mZoomMode; }
public boolean isNativeMode() {
return nativeMode;
}

public int getZoomMode() {
return mZoomMode;
}

public void setZoom(int mode) {
mZoomMode = mode;
Expand Down Expand Up @@ -175,7 +181,7 @@ public void setMetaDuration(long duration) {
}

public long getDuration() {
if (nativeMode){
if (nativeMode) {
return mExoPlayer.getDuration() > 0 ? mExoPlayer.getDuration() : mMetaDuration;
} else {
return mVlcPlayer.getLength() > 0 ? mVlcPlayer.getLength() : mMetaDuration;
Expand Down Expand Up @@ -285,13 +291,14 @@ public long seekTo(long pos) {
mLastTime = mVlcPlayer.getTime();
Timber.i("VLC length in seek is: %d", mVlcPlayer.getLength());
try {
if (getDuration() > 0) mVlcPlayer.setPosition((float)pos / getDuration()); else mVlcPlayer.setTime(pos);
if (getDuration() > 0) mVlcPlayer.setPosition((float) pos / getDuration());
else mVlcPlayer.setTime(pos);

return pos;

} catch (Exception e) {
Timber.e(e, "Error seeking in VLC");
Utils.showToast(mActivity, mActivity.getString(R.string.seek_error));
Utils.showToast(mActivity, mActivity.getString(R.string.seek_error));
return -1;
}
}
Expand Down Expand Up @@ -349,7 +356,7 @@ public boolean setSubtitleTrack(int index, @Nullable List<MediaStream> allStream
} catch (IndexOutOfBoundsException e) {
Timber.e("Could not locate subtitle with index %s in vlc track info", index);
return false;
} catch (NullPointerException e){
} catch (NullPointerException e) {
Timber.e("No subtitle tracks found in player trying to set subtitle with index %s in vlc track info", index);
return false;
}
Expand Down Expand Up @@ -387,7 +394,7 @@ public void setAudioTrack(int ndx, List<MediaStream> allStreams) {
Timber.e("Could not locate audio with index %s in vlc track info", ndx);
mVlcPlayer.setAudioTrack(ndx);
return;
} catch (NullPointerException e){
} catch (NullPointerException e) {
Timber.e("No subtitle tracks found in player trying to set subtitle with index %s in vlc track info", ndx);
mVlcPlayer.setAudioTrack(vlcIndex);
return;
Expand All @@ -409,8 +416,8 @@ public void setAudioTrack(int ndx, List<MediaStream> allStreams) {
}
}

public void setPlaybackSpeed(Double speed){
if (nativeMode){
public void setPlaybackSpeed(@NonNull Double speed) {
if (nativeMode) {
mExoPlayer.setPlaybackSpeed(speed.floatValue());
} else {
mVlcPlayer.setRate(speed.floatValue());
Expand Down Expand Up @@ -445,7 +452,7 @@ public void setAudioMode() {
}

private void setVlcAudioOptions() {
if(!Utils.downMixAudio()) {
if (!Utils.downMixAudio()) {
mVlcPlayer.setAudioDigitalOutputEnabled(true);
} else {
setCompatibleAudio();
Expand Down Expand Up @@ -552,7 +559,7 @@ public void contractVideo(int height) {
Activity activity = TvApp.getApplication().getCurrentActivity();
int sw = activity.getWindow().getDecorView().getWidth();
int sh = activity.getWindow().getDecorView().getHeight();
float ar = (float)sw / sh;
float ar = (float) sw / sh;
lp.height = height;
lp.width = (int) Math.ceil(height * ar);
lp.rightMargin = ((lp.width - normalWidth) / 2) - 110;
Expand Down Expand Up @@ -618,10 +625,10 @@ private void changeSurfaceLayout(int videoWidth, int videoHeight, int videoVisib
double ar;
if (sarDen == sarNum) {
/* No indication about the density, assuming 1:1 */
ar = (double)videoVisibleWidth / (double)videoVisibleHeight;
ar = (double) videoVisibleWidth / (double) videoVisibleHeight;
} else {
/* Use the specified aspect ratio */
double vw = videoVisibleWidth * (double)sarNum / sarDen;
double vw = videoVisibleWidth * (double) sarNum / sarDen;
ar = vw / videoVisibleHeight;
}

Expand All @@ -635,7 +642,7 @@ private void changeSurfaceLayout(int videoWidth, int videoHeight, int videoVisib

// set display size
ViewGroup.LayoutParams lp = mSurfaceView.getLayoutParams();
lp.width = (int) Math.ceil(dw * videoWidth / videoVisibleWidth);
lp.width = (int) Math.ceil(dw * videoWidth / videoVisibleWidth);
lp.height = (int) Math.ceil(dh * videoHeight / videoVisibleHeight);
normalWidth = lp.width;
normalHeight = lp.height;
Expand Down Expand Up @@ -679,6 +686,7 @@ public void setOnProgressListener(PlaybackListener listener) {

private PlaybackListener progressListener;
private Runnable progressLoop;

private void startProgressLoop() {
progressLoop = new Runnable() {
@Override
Expand Down Expand Up @@ -724,7 +732,7 @@ public void onNewVideoLayout(IVLCVout vout, int width, int height, int visibleWi
mVideoHeight = height;
mVideoWidth = width;
mVideoVisibleHeight = visibleHeight;
mVideoVisibleWidth = visibleWidth;
mVideoVisibleWidth = visibleWidth;
mSarNum = sarNum;
mSarDen = sarDen;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,32 @@ package org.jellyfin.androidtv.ui.playback

class VideoSpeedController(playbackController: PlaybackController) {
companion object {
enum class SpeedSteps(val value: Double) {
SPEED_0_25(0.25),
SPEED_0_50(0.5),
SPEED_0_75(0.75),
SPEED_1_00(1.0),
SPEED_1_25(1.25),
SPEED_1_50(1.50),
SPEED_1_75(1.75),
SPEED_2_00(2.0),
enum class SpeedSteps(val speed: Double) {
// Use named parameter so detekt knows these aren't magic values
SPEED_0_25(speed = 0.25),
SPEED_0_50(speed = 0.5),
SPEED_0_75(speed = 0.75),
SPEED_1_00(speed = 1.0),
SPEED_1_25(speed = 1.25),
SPEED_1_50(speed = 1.50),
SPEED_1_75(speed = 1.75),
SPEED_2_00(speed = 2.0),
}

private var previousSpeedSelection = SpeedSteps.SPEED_1_00
fun resetPreviousSpeedToDefault(){
fun resetPreviousSpeedToDefault() {
previousSpeedSelection = SpeedSteps.SPEED_1_00
}
}

private val parentController = playbackController

init {
// Carry forward the user's recent speed selection onto the next video(s)
setNewSpeed(previousSpeedSelection)
}

fun getCurrentSpeed(): SpeedSteps{
fun getCurrentSpeed(): SpeedSteps {
// Currently getCurrentSpeed uses previousSpeedSelection (from the companion)
// but this is an implementation detail I'd rather not leak in-case we ever need
// to separate out the two details. So implement a custom named getter...
Expand All @@ -33,6 +36,6 @@ class VideoSpeedController(playbackController: PlaybackController) {

fun setNewSpeed(selectedSpeed: SpeedSteps) {
previousSpeedSelection = selectedSpeed
parentController.setPlaybackSpeed(selectedSpeed.value)
parentController.setPlaybackSpeed(selectedSpeed.speed)
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package org.jellyfin.androidtv.ui.playback.overlay.action

import android.content.Context
import org.jellyfin.androidtv.ui.playback.overlay.CustomPlaybackTransportControlGlue
import org.jellyfin.androidtv.ui.playback.PlaybackController
import org.jellyfin.androidtv.ui.playback.overlay.LeanbackOverlayFragment
import android.view.Gravity
import android.view.MenuItem
import android.view.View
import android.widget.PopupMenu
import org.jellyfin.androidtv.R
import org.jellyfin.androidtv.ui.playback.PlaybackController
import org.jellyfin.androidtv.ui.playback.VideoSpeedController
import org.jellyfin.androidtv.ui.playback.overlay.CustomPlaybackTransportControlGlue
import org.jellyfin.androidtv.ui.playback.overlay.LeanbackOverlayFragment
import java.util.*

class PlaybackSpeedAction(
context: Context,
Expand Down Expand Up @@ -48,8 +48,9 @@ class PlaybackSpeedAction(
): PopupMenu {
val speedMenu = PopupMenu(context, view, Gravity.END)
val menu = speedMenu.menu
speeds.forEachIndexed { i, speed ->
menu.add(0, i, i, String.format("%.2fx", speed.value))
speeds.forEachIndexed { i, selected ->
// Since this is purely numeric data, coerce to en_us to keep the linter happy
menu.add(0, i, i, String.format(Locale.US, "%.2fx", selected.speed))
}

menu.setGroupCheckable(0, true, true)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package org.jellyfin.androidtv.ui.playback

import io.mockk.*
import io.mockk.justRun
import io.mockk.mockk
import io.mockk.slot
import io.mockk.verify
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Test
import org.junit.Assert.*
import kotlin.math.exp

class VideoSpeedControllerTest {
@After
Expand All @@ -19,7 +21,7 @@ class VideoSpeedControllerTest {
val expectedStep = 0.25
var i = 1
speedSteps.forEach { v ->
assertEquals(i * expectedStep, v.value, 0.001)
assertEquals(i * expectedStep, v.speed, 0.001)
i += 1
}
}
Expand All @@ -37,7 +39,7 @@ class VideoSpeedControllerTest {
}

@Test
fun testSetNewSpeed(){
fun testSetNewSpeed() {
val mockController = mockk<PlaybackController>(relaxed = true)
val controller = VideoSpeedController(mockController)
val expected = VideoSpeedController.Companion.SpeedSteps.SPEED_1_25
Expand All @@ -56,7 +58,7 @@ class VideoSpeedControllerTest {
controller.setNewSpeed(expected)

verify { mockController.setPlaybackSpeed(any()) }
assertEquals(expected.value, slot.captured, 0.0001)
assertEquals(expected.speed, slot.captured, 0.0001)
}

@Test
Expand All @@ -70,7 +72,11 @@ class VideoSpeedControllerTest {
VideoSpeedController(mockController)

verify { mockController.setPlaybackSpeed(any()) }
assertEquals(VideoSpeedController.Companion.SpeedSteps.SPEED_1_00.value, slot.captured, 0.0001)
assertEquals(
VideoSpeedController.Companion.SpeedSteps.SPEED_1_00.speed,
slot.captured,
0.0001
)
}


Expand All @@ -90,7 +96,7 @@ class VideoSpeedControllerTest {
assertEquals(lastSetSpeed, slot.captured, 0.001)

controller.setNewSpeed(newSpeed)
lastSetSpeed = newSpeed.value
lastSetSpeed = newSpeed.speed
}
}

Expand Down

0 comments on commit a25c7e2

Please sign in to comment.