feat: Add Title and Summary to video overlay of playing video#512
Merged
kingargyle merged 1 commit intodevelopfrom Jan 19, 2026
Merged
feat: Add Title and Summary to video overlay of playing video#512kingargyle merged 1 commit intodevelopfrom
kingargyle merged 1 commit intodevelopfrom
Conversation
Adds the Title and Summary information to the video overlay. For series it adds the Series Name and the Episode title along with the summary.
Contributor
WalkthroughThis PR adds video metadata display functionality to the ExoPlayer interface. It introduces a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #512 +/- ##
=============================================
+ Coverage 48.61% 48.63% +0.02%
- Complexity 473 474 +1
=============================================
Files 65 65
Lines 2199 2204 +5
Branches 309 312 +3
=============================================
+ Hits 1069 1072 +3
+ Misses 1002 1001 -1
- Partials 128 131 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@serenity-app/src/main/kotlin/us/nineworlds/serenity/ui/video/player/ExoplayerPresenter.kt`:
- Around line 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.
In
`@serenity-app/src/main/kotlin/us/nineworlds/serenity/ui/video/player/ExoplayerVideoActivity.kt`:
- Around line 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).
In
`@serenity-app/src/test/kotlin/us/nineworlds/serenity/ui/video/player/ExoplayerPresenterTest.kt`:
- Around line 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.
🧹 Nitpick comments (1)
serenity-app/src/main/res/layout/exo_player_control_view.xml (1)
13-39: Guard long titles to prevent overlay collisions.Consider adding
maxLinesandellipsizeonvideo_title(similar to summary) so very long titles don’t overlap other controls.🔧 Suggested tweak
<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" /> + android:textStyle="bold" + android:maxLines="2" + android:ellipsize="end" />
Comment on lines
162
to
166
| 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) |
Contributor
There was a problem hiding this comment.
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.
Comment on lines
+403
to
+409
| 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 | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ 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).
Comment on lines
+193
to
205
| 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) } |
Contributor
There was a problem hiding this comment.
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.
Contributor
Author
|
Close #387 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the Title and Summary information to the video overlay. For series it adds the Series Name and the Episode title along with the summary.
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.