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 @@ -41,6 +41,9 @@ interface ExoplayerContract {

@StateStrategyType(AddToEndSingleStrategy::class)
fun toggleDebugView()

@StateStrategyType(AddToEndSingleStrategy::class)
fun setVideoInfo(title: String?, summary: String?)
}

interface ExoplayerPresenter {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import us.nineworlds.serenity.common.annotations.OpenForTesting
import us.nineworlds.serenity.common.rest.SerenityClient
import us.nineworlds.serenity.core.logger.Logger
import us.nineworlds.serenity.core.model.VideoContentInfo
import us.nineworlds.serenity.core.model.impl.EpisodePosterInfo
import us.nineworlds.serenity.core.util.AndroidHelper
import us.nineworlds.serenity.events.video.OnScreenDisplayEvent
import us.nineworlds.serenity.injection.ForVideoQueue
Expand Down Expand Up @@ -161,6 +162,7 @@ class ExoplayerPresenter :
override fun playVideo() {
val videoUrl: String = transcoderUrl()
startPlaying()
viewState.setVideoInfo(if (video.seriesName != null) "${video.seriesName}\n${video.getTitle()}" else video.getTitle(), video.getSummary())
viewState.updateSerenityDebugInfo(isTranscoding, video.videoCodec, video.audioCodec, 0)
Comment on lines 162 to 166
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 | 🟡 Minor

Avoid displaying literal "null" or stray newline in the title.

If video.getTitle() is null (or blank), the current interpolation can render "null" or an empty line. Consider normalizing title/series values before building the display string.

🔧 Suggested fix
-        viewState.setVideoInfo(if (video.seriesName != null) "${video.seriesName}\n${video.getTitle()}" else video.getTitle(), video.getSummary())
+        val displayTitle = if (!video.seriesName.isNullOrBlank()) {
+            listOfNotNull(video.seriesName, video.getTitle()).joinToString("\n")
+        } else {
+            video.getTitle().orEmpty()
+        }
+        viewState.setVideoInfo(displayTitle, video.getSummary())
🤖 Prompt for AI Agents
In
`@serenity-app/src/main/kotlin/us/nineworlds/serenity/ui/video/player/ExoplayerPresenter.kt`
around lines 162 - 166, Normalize the series and title before formatting to
avoid literal "null" or stray newlines: in playVideo(), fetch video.seriesName
and video.getTitle() into local vars, trim and treat null/blank as empty (or a
fallback like an empty string), then build the display string by conditionally
joining non-empty parts (e.g., only prepend seriesName + "\n" when seriesName is
non-empty) and pass that sanitized string to viewState.setVideoInfo along with
video.getSummary(); update references to playVideo(), viewState.setVideoInfo,
video.seriesName, and video.getTitle() accordingly.

viewState.initializePlayer(videoUrl, video.resumeOffset)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,14 @@ class ExoplayerVideoActivity :
}
}

override fun setVideoInfo(title: String?, summary: String?) {
val videoTitle = playerView.findViewById<TextView>(R.id.video_title)
val videoSummary = playerView.findViewById<TextView>(R.id.video_summary)

videoTitle?.text = title
videoSummary?.text = summary
}
Comment on lines +403 to +409
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

Replace findViewById with View Binding for control views.

This Activity should avoid direct findViewById access. Prefer ExoPlayerControlViewBinding (or equivalent) and reuse it for title/summary updates. As per coding guidelines, use View Binding in Activities.

♻️ Suggested change (within this method)
-        val videoTitle = playerView.findViewById<TextView>(R.id.video_title)
-        val videoSummary = playerView.findViewById<TextView>(R.id.video_summary)
-
-        videoTitle?.text = title
-        videoSummary?.text = summary
+        val controlsBinding = ExoPlayerControlViewBinding.bind(
+            playerView.findViewById(R.id.exo_controller)
+        )
+        controlsBinding.videoTitle.text = title
+        controlsBinding.videoSummary.text = summary
🤖 Prompt for AI Agents
In
`@serenity-app/src/main/kotlin/us/nineworlds/serenity/ui/video/player/ExoplayerVideoActivity.kt`
around lines 403 - 409, In setVideoInfo, stop calling playerView.findViewById
and instead use a ViewBinding instance for the ExoPlayer control layout (e.g.,
ExoPlayerControlViewBinding) that you initialize/reuse as a property; replace
lookups of R.id.video_title and R.id.video_summary with
controlBinding.videoTitle.text = title and controlBinding.videoSummary.text =
summary (ensure controlBinding is created from playerView or inflated once and
reused so setVideoInfo only updates the binding fields).


class SubtitleTrackNameProvider(val resources: Resources) : TrackNameProvider {
override fun getTrackName(format: Format): String {
val lang = format.language
Expand Down
29 changes: 29 additions & 0 deletions serenity-app/src/main/res/layout/exo_player_control_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,35 @@
android:layout_height="match_parent"
android:background="#CC000000" />

<LinearLayout
android:id="@+id/video_info_container"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_gravity="top"
android:orientation="vertical"
android:paddingStart="@dimen/overscan_start_padding"
android:paddingEnd="@dimen/overscan_end_padding"
android:paddingTop="@dimen/overscan_top_padding"
android:background="#80000000"
>
<TextView
android:id="@+id/video_title"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textColor="@android:color/white"
android:textSize="24sp"
android:textStyle="bold" />

<TextView
android:id="@+id/video_summary"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textColor="@android:color/white"
android:textSize="16sp"
android:maxLines="3"
android:ellipsize="end" />
</LinearLayout>

<FrameLayout
android:id="@id/exo_center_controls"
android:layout_width="wrap_content"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,13 +190,17 @@ class ExoplayerPresenterTest : InjectingTest() {
videoContentInfo.videoCodec = "h264"
videoContentInfo.audioCodec = "ac3"
videoContentInfo.directPlayUrl = "http://direct.url"
videoContentInfo.seriesTitle = null
videoContentInfo.setTitle("Title")
videoContentInfo.setSummary("Summary")
presenter.video = videoContentInfo

every { presenter.isDirectPlaySupportedForContainer(any()) } returns false
every { mockSerenityClient.createTranscodeUrl(any(), any()) } returns "http://transcode.url"

presenter.playVideo()

verify { mockView.setVideoInfo("Title", "Summary") }
verify { mockView.updateSerenityDebugInfo(true, "h264", "ac3", 0) }
verify { mockView.initializePlayer("http://transcode.url", 0) }
Comment on lines +193 to 205
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 | 🟡 Minor

Add coverage for the series-title formatting branch.

Current tests only cover seriesTitle == null. Please add a case where seriesTitle is non-null to assert the "Series\nTitle" formatting for the overlay. As per coding guidelines, new feature branches should be covered by tests.

🤖 Prompt for AI Agents
In
`@serenity-app/src/test/kotlin/us/nineworlds/serenity/ui/video/player/ExoplayerPresenterTest.kt`
around lines 193 - 205, Add a new test case in ExoplayerPresenterTest that sets
videoContentInfo.seriesTitle to a non-null value (e.g., "Series Title"), assigns
presenter.video = videoContentInfo, stubs the same dependencies
(isDirectPlaySupportedForContainer and mockSerenityClient.createTranscodeUrl),
calls presenter.playVideo(), and then verify that mockView.setVideoInfo is
invoked with a title argument that contains the formatted series title
"Series\nTitle" (use a matcher/predicate in verify to assert the string contains
the newline-formatted series title) in addition to verifying the other expected
view calls (updateSerenityDebugInfo and initializePlayer) as in the existing
test.

}
Expand All @@ -207,12 +211,15 @@ class ExoplayerPresenterTest : InjectingTest() {
videoContentInfo.videoCodec = "h264"
videoContentInfo.audioCodec = "ac3"
videoContentInfo.directPlayUrl = "http://direct.url"
videoContentInfo.setTitle("Title")
videoContentInfo.setSummary("Summary")
presenter.video = videoContentInfo

every { presenter.isDirectPlaySupportedForContainer(any()) } returns true

presenter.playVideo()

verify { mockView.setVideoInfo("Title", "Summary") }
verify { mockView.updateSerenityDebugInfo(false, "h264", "ac3", 0) }
verify { mockView.initializePlayer("http://direct.url", 0) }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,20 @@ open class ExoplayerVideoActivityTest : InjectingTest() {
}
}

@Test
fun setVideoInfoUpdatesTitleAndSummary() {
val expectedTitle = "Test Title"
val expectedSummary = "Test Summary"

activity.setVideoInfo(expectedTitle, expectedSummary)

val videoTitle = activity.playerView.findViewById<TextView>(R.id.video_title)
val videoSummary = activity.playerView.findViewById<TextView>(R.id.video_summary)

assertThat(videoTitle.text.toString()).isEqualTo(expectedTitle)
assertThat(videoSummary.text.toString()).isEqualTo(expectedSummary)
}

override fun installTestModules() {
Toothpick.openScope(InjectionConstants.APPLICATION_SCOPE).installTestModules(object : Module() {
init {
Expand All @@ -355,7 +369,7 @@ open class ExoplayerVideoActivityTest : InjectingTest() {
scope.installTestModules(MockkTestingModule(), TestModule())
}

inner class TestModule : Module() {
class TestModule : Module() {

init {
bind(ExoplayerPresenter::class.java).toInstance(mockExoPlayerPresenter)
Expand Down