Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,14 @@ import us.nineworlds.serenity.emby.moshi.LocalDateJsonAdapter
import us.nineworlds.serenity.emby.server.model.AuthenticateUserByName
import us.nineworlds.serenity.emby.server.model.AuthenticationResult
import us.nineworlds.serenity.emby.server.model.Item
import us.nineworlds.serenity.emby.server.model.PlaybackStartRequest
import us.nineworlds.serenity.emby.server.model.PublicUserInfo
import us.nineworlds.serenity.emby.server.model.QueryFilters
import us.nineworlds.serenity.emby.server.model.QueryResult
import java.io.File
import java.io.IOException
import java.util.UUID
import kotlin.uuid.Uuid
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove unused import kotlin.uuid.Uuid.

This import is not used in the file. The code uses java.util.UUID throughout.

🔎 Proposed fix
-import kotlin.uuid.Uuid
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import kotlin.uuid.Uuid
🤖 Prompt for AI Agents
In
emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/server/api/EmbyAPIClient.kt
around line 34, remove the unused import "import kotlin.uuid.Uuid" because the
file consistently uses java.util.UUID; simply delete that import line to
eliminate the unused import warning and rely on the existing java.util.UUID
usages.


class EmbyAPIClient(val context: Context, baseUrl: String = "http://localhost:8096") : SerenityClient {

Expand All @@ -53,7 +55,7 @@ class EmbyAPIClient(val context: Context, baseUrl: String = "http://localhost:80
val cache = Cache(cacheDir, cacheSize.toLong())

val okClient = RetrofitUrlManager.getInstance().with(OkHttpClient.Builder())
logger.level = HttpLoggingInterceptor.Level.BASIC
logger.level = HttpLoggingInterceptor.Level.BODY
okClient.addInterceptor(logger)
Comment on lines 57 to 59
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Logging level BODY may expose sensitive data.

Changing from BASIC to BODY logging will log full request and response bodies, which could include sensitive information (tokens, user data). Consider keeping this at BASIC or HEADERS for production builds, or gating behind a debug flag.

🔎 Proposed fix
-logger.level = HttpLoggingInterceptor.Level.BODY
+logger.level = if (BuildConfig.DEBUG) {
+    HttpLoggingInterceptor.Level.BODY
+} else {
+    HttpLoggingInterceptor.Level.BASIC
+}
🤖 Prompt for AI Agents
In
emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/server/api/EmbyAPIClient.kt
around lines 57-59, the logging level is set to BODY which can expose sensitive
request/response payloads; change this to a safer default
(HttpLoggingInterceptor.Level.BASIC or HEADERS) and/or gate BODY behind a debug
flag (e.g., BuildConfig.DEBUG) so full bodies are logged only in debug builds;
update the code to set logger.level conditionally and keep the production
default to BASIC/HEADERS.

okClient.cache(cache)

Expand Down Expand Up @@ -412,14 +414,18 @@ class EmbyAPIClient(val context: Context, baseUrl: String = "http://localhost:80
}

override fun progress(key: String, offset: String): Boolean {
return progress(key, offset, null)
}

override fun progress(key: String, offset: String, playSessionId: String?): Boolean {
if (userId == null) {
userId = fetchUserId()
}
var position = offset.toLong()

position = position.times(10000)

val call = usersService.progress(headerMap(), userId!!, key, null, position)
val call = usersService.progress(headerMap(), userId!!, key, null, position, playSessionId)
val result = call.execute()

return result.isSuccessful
Expand Down Expand Up @@ -475,14 +481,18 @@ class EmbyAPIClient(val context: Context, baseUrl: String = "http://localhost:80
return "$baseUrl/Users/${user.userId}/Images/Primary?Width=$width&Height=$height"
}

override fun startPlaying(itemId: String) {
override fun startPlaying(itemId: String): String? {
if (userId == null) {
userId = fetchUserId()
}

val call = usersService.startPlaying(headerMap(), userId!!, itemId)
val call = usersService.startPlaying(headerMap(), userId!!, itemId, PlaybackStartRequest(true))

call.execute()
val response = call.execute()
if (response.isSuccessful) {
return response.body()?.playSessionId ?: UUID.randomUUID().toString()
}
return UUID.randomUUID().toString()
}

override fun stopPlaying(itemId: String, offset: Long) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,24 +114,27 @@ interface UsersService {
@Path("userId") userId: String,
@Path("itemId") itemId: String,
@Query("MediaSourceId") mediaSourceId: String? = null,
@Query("PositionTicks") positionTicks: Long
@Query("PositionTicks") positionTicks: Long,
@Query("PlaySessionId") playSessionId: String? = null
): Call<Void>

@POST("/emby/Users/{userId}/PlayingItems/{itemId}")
fun startPlaying(
@HeaderMap headerMap: Map<String, String>,
@Path("userId") userId: String,
@Path("itemId") itemId: String,
@Body playbackStartRequest: PlaybackStartRequest,
@Query("MediaSourceId") mediaSourceId: String? = null
): Call<Void>
): Call<PlaybackStartInfo>

@DELETE("/emby/Users/{userId}/PlayingItems/{itemId}")
fun stopPlaying(
@HeaderMap headerMap: Map<String, String>,
@Path("userId") userId: String,
@Path("itemId") itemId: String,
@Query("MediaSourceId") mediaSourceId: String? = null,
@Query("PositionTicks") positionTicks: String
@Query("PositionTicks") positionTicks: String,
@Query("PlaySessionId") playSessionId: String? = null
): Call<Void>

@GET("/emby/Search/Hints")
Expand All @@ -154,4 +157,4 @@ interface UsersService {
@Query("Limit") limit: Int? = 25,
): Call<QueryResult>

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,18 @@ data class SearchHint(
@Json(name = "Series") val series: String?,
@Json(name = "Status") val status: String?,
@Json(name = "Type") val type: String?
)
)

data class PlaybackStartInfo(
@Json(name = "QueueItem") val queueItem: QueueItem?,
@Json(name = "PlaySessionId") val playSessionId: String?
)

data class QueueItem(
@Json(name = "Id") val id: Long?,
@Json(name = "PlaylistItemId") val playlistItemId: String?
)

data class PlaybackStartRequest(
@Json(name = "CanSeek") val canSeek: Boolean = true
)
4 changes: 3 additions & 1 deletion gradle/libs.versions.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ leanbackVersion = "1.2.0"
legacySupportV4Version = "1.0.0"
lifecycleVersion = "2.9.4"
materialVersion = "1.12.0"
minSdkVersion = "27"
minSdkVersion = "25"
mockwebserverVersion = "4.9.1"
moxy = "2.2.2"
moxyAppCompatVersion = "2.2.2"
Expand Down Expand Up @@ -61,6 +61,7 @@ agpVersion = "8.13.1"
kotlin = "2.2.21"
coreKtxVersion = "1.17.0"
espressoCoreVersion = "3.5.1"
lifecycleProcessVersion = "2.10.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

lifecycle-process version 2.10.0 does not exist—use an available version instead.

The latest stable release of lifecycle-process is 2.8.6, with 2.9.0-alpha03 as the latest pre-release. Version 2.10.0 is unavailable for this artifact and will cause a dependency resolution failure during the build. Change lifecycleProcessVersion to 2.8.6 (stable) or 2.9.0-alpha03 if pre-release versions are acceptable.

Also applies to: 144-144

🤖 Prompt for AI Agents
In gradle/libs.versions.toml at line 64 (and also at line 144), the
lifecycleProcessVersion value "2.10.0" is invalid and will break dependency
resolution; change the value to a valid release such as "2.8.6" (stable) or
"2.9.0-alpha03" (pre-release) and ensure any other occurrences (line 144) are
updated consistently.


[libraries]
androidx-annotation = { module = "androidx.annotation:annotation", version.ref = "annotationVersion" }
Expand Down Expand Up @@ -140,6 +141,7 @@ assertk-jvm = { module = "com.willowtreeapps.assertk:assertk", version.ref = "as
turbine = { module = "app.cash.turbine:turbine", version.ref = "turbine"}
androidx-core-ktx = { group = "androidx.core", name = "core-ktx", version.ref = "coreKtxVersion" }
androidx-espresso-core = { group = "androidx.test.espresso", name = "espresso-core", version.ref = "espressoCoreVersion" }
androidx-lifecycle-process = { group = "androidx.lifecycle", name = "lifecycle-process", version.ref = "lifecycleProcessVersion" }

[plugins]
ksp = { id = "com.google.devtools.ksp", version.ref = "kotlinKSPVersion" }
Expand Down
3 changes: 2 additions & 1 deletion serenity-app/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ dependencies {
implementation(libs.moxy.community.moxy)
implementation(libs.moxy.ktx)
implementation(libs.github.glide)
ksp(libs.glide.compiler)
implementation(libs.androidx.lifecycle.process)
ksp(libs.glide.compiler)
Comment on lines +148 to +149
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Fix incorrect indentation.

Lines 148-149 have 4 extra spaces of indentation compared to the rest of the dependencies block, making them inconsistent with the file's formatting.

🔎 Proposed fix
-    implementation(libs.androidx.lifecycle.process)
-    ksp(libs.glide.compiler)
+  implementation(libs.androidx.lifecycle.process)
+  ksp(libs.glide.compiler)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
implementation(libs.androidx.lifecycle.process)
ksp(libs.glide.compiler)
implementation(libs.androidx.lifecycle.process)
ksp(libs.glide.compiler)
🤖 Prompt for AI Agents
In serenity-app/build.gradle.kts around lines 148-149, the two dependency lines
("implementation(libs.androidx.lifecycle.process)" and
"ksp(libs.glide.compiler)") have 4 extra leading spaces making them misaligned
with the rest of the dependencies block; fix by removing the 4 extra spaces so
both lines use the same indentation level as the surrounding dependency entries.

implementation(libs.androidx.appcompat)
implementation(libs.androidx.leanback)
implementation(libs.androidx.leanback.preference)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ import android.content.SharedPreferences
import android.os.Build
import android.os.Bundle
import android.preference.PreferenceManager
import androidx.lifecycle.ProcessLifecycleOwner
import androidx.lifecycle.lifecycleScope
import androidx.localbroadcastmanager.content.LocalBroadcastManager
import com.google.android.exoplayer2.database.ExoDatabaseProvider
import com.google.android.exoplayer2.upstream.cache.LeastRecentlyUsedCacheEvictor
import com.google.android.exoplayer2.upstream.cache.SimpleCache
import com.google.firebase.analytics.FirebaseAnalytics
import kotlinx.coroutines.MainScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import net.danlew.android.joda.JodaTimeAndroid
import org.greenrobot.eventbus.EventBus
Expand Down Expand Up @@ -141,7 +143,7 @@ open class SerenityApplication : Application() {
}

protected open fun discoverServers() {
MainScope().launch {
ProcessLifecycleOwner.get().lifecycleScope.launch(Dispatchers.IO) {
EmbyServerDiscover().findServers()
}
}
Comment on lines 145 to 149
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for build.gradle files containing lifecycle-process dependency
find . -type f \( -name "build.gradle" -o -name "build.gradle.kts" \) | head -20

Repository: NineWorlds/serenity-android

Length of output: 348


🏁 Script executed:

#!/bin/bash
# Search for lifecycle-process in all build.gradle files
echo "=== Searching for lifecycle-process ==="
grep -r "lifecycle-process" . --include="*.gradle" --include="*.gradle.kts" 2>/dev/null || echo "Not found in build files"

echo -e "\n=== Searching for androidx.lifecycle in serenity-app/build.gradle.kts ==="
grep -E "androidx\.lifecycle|lifecycle" serenity-app/build.gradle.kts 2>/dev/null || echo "Not found"

echo -e "\n=== Full serenity-app/build.gradle.kts dependencies section ==="
sed -n '/^dependencies/,/^}/p' serenity-app/build.gradle.kts 2>/dev/null | head -100

Repository: NineWorlds/serenity-android

Length of output: 3515


🏁 Script executed:

#!/bin/bash
# Read the SerenityApplication.kt file, especially around lines 145-149
echo "=== SerenityApplication.kt (lines 140-160) ==="
sed -n '140,160p' serenity-app/src/main/java/us/nineworlds/serenity/SerenityApplication.kt

echo -e "\n=== Import statements in SerenityApplication.kt ==="
head -30 serenity-app/src/main/java/us/nineworlds/serenity/SerenityApplication.kt

echo -e "\n=== Check for Toothpick usage in SerenityApplication.kt ==="
grep -E "Toothpick|inject|Inject" serenity-app/src/main/java/us/nineworlds/serenity/SerenityApplication.kt || echo "No Toothpick usage found"

Repository: NineWorlds/serenity-android

Length of output: 2791


Lifecycle-aware coroutine management is solid, but dependency injection pattern needs adjustment.

The lifecycle-process dependency is properly declared, and using ProcessLifecycleOwner.get().lifecycleScope.launch(Dispatchers.IO) correctly ties the server discovery coroutine to the app process lifecycle while offloading IO work. However, EmbyServerDiscover() should be injected via Toothpick rather than instantiated directly, following the project's DI guidelines used elsewhere in SerenityApplication.kt.

🤖 Prompt for AI Agents
In serenity-app/src/main/java/us/nineworlds/serenity/SerenityApplication.kt
around lines 145-149, replace the direct instantiation EmbyServerDiscover() with
a Toothpick-injected instance: obtain the EmbyServerDiscover from the
application (or appropriate) Toothpick scope and call its findServers() inside
the same lifecycleScope coroutine; ensure the EmbyServerDiscover binding is
registered in the Toothpick module and that Toothpick is initialized/imported in
this file.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,35 @@ object UpdateProgressRequestJob {
}
}

fun updateProgress(position: Long, video: VideoContentInfo, playSessionId: String?) {
if (scope == null) {
job = SupervisorJob()
scope = CoroutineScope(Dispatchers.IO + job!!)
}

scope?.launch {
val id = video.id()
if (id == null) {
Timber.w("Video ID is null. Cannot update progress for video: ${video.id()}")
return@launch
}

try {
if (video.isWatched) {
serenityClient.watched(id)
serenityClient.progress(id, "0", playSessionId)
} else {
serenityClient.progress(id, position.toString(), playSessionId)
}
} catch (e: Exception) {
if (e is CancellationException) {
return@launch
}
Timber.e(e, "Error updating progress")
}
}
}

fun onFinish() {
scope?.cancel()
scope = null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class ExoplayerPresenter : MvpPresenter<ExoplayerView>(), ExoplayerPresenter,
internal lateinit var video: VideoContentInfo

private var onScreenControllerShowing: Boolean = false
private var playSessionId: String? = null

override fun attachView(view: ExoplayerView?) {
Toothpick.inject(this, Toothpick.openScope(InjectionConstants.APPLICATION_SCOPE))
Expand Down Expand Up @@ -97,11 +98,15 @@ class ExoplayerPresenter : MvpPresenter<ExoplayerView>(), ExoplayerPresenter,
presenterScope.launch {
playbackRepository.stopPlaying(video.id().orEmpty(), currentPosition)
}
playSessionId = null
}

override fun startPlaying() {
presenterScope.launch {
playbackRepository.startPlaying(video.id().orEmpty())

if (playSessionId == null) {
playSessionId = playbackRepository.startPlaying(video.id().orEmpty())
}
}
}

Expand Down Expand Up @@ -131,7 +136,7 @@ class ExoplayerPresenter : MvpPresenter<ExoplayerView>(), ExoplayerPresenter,
override fun updateServerPlaybackPosition(currentPostion: Long) {
presenterScope.launch {
video.resumeOffset = currentPostion.toInt()
playbackRepository.updatePlaybackPosition(video)
playbackRepository.updatePlaybackPosition(video, playSessionId)
}
}

Expand All @@ -152,7 +157,7 @@ class ExoplayerPresenter : MvpPresenter<ExoplayerView>(), ExoplayerPresenter,

override fun playVideo() {
val videoUrl: String = transcoderUrl()

startPlaying()
viewState.initializePlayer(videoUrl, video.resumeOffset)
}

Expand Down Expand Up @@ -185,4 +190,4 @@ class ExoplayerPresenter : MvpPresenter<ExoplayerView>(), ExoplayerPresenter,

private fun selectCodec(mimeType: String): Boolean =
MediaCodecInfoUtil.isCodecSupported(mimeType)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,35 @@ package us.nineworlds.serenity.ui.video.player

import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import timber.log.Timber
import toothpick.InjectConstructor
import us.nineworlds.serenity.common.rest.SerenityClient
import us.nineworlds.serenity.core.model.VideoContentInfo

@InjectConstructor
class PlaybackRepository(private val serenityClient: SerenityClient) {

suspend fun startPlaying(videoId: String) = withContext(Dispatchers.IO) {
serenityClient.startPlaying(videoId)
suspend fun startPlaying(videoId: String): String? = withContext(Dispatchers.IO) {
val playSessionId = serenityClient.startPlaying(videoId)
Timber.d("Playback Session Id: ${playSessionId}")
playSessionId
}

suspend fun stopPlaying(videoId: String, offset: Long) = withContext(Dispatchers.IO){
serenityClient.stopPlaying(videoId, offset)
}

suspend fun updatePlaybackPosition(video: VideoContentInfo) = withContext(Dispatchers.IO){
suspend fun updatePlaybackPosition(video: VideoContentInfo, playSessionId: String? = null) = withContext(Dispatchers.IO){
val videoId: String = video.id().orEmpty()
if (video.isWatched) {
serenityClient.watched(videoId)
serenityClient.progress(videoId, "0")
serenityClient.progress(videoId, "0", playSessionId)
} else {
serenityClient.progress(videoId, video.resumeOffset.toString())
serenityClient.progress(videoId, video.resumeOffset.toString(), playSessionId)
}
}

suspend fun watched(videoId: String) = withContext(Dispatchers.IO) {
serenityClient.watched(videoId)
}


}
}
Loading