feat(directplay): Intelligently select best media combination for pla…#491
feat(directplay): Intelligently select best media combination for pla…#491kingargyle merged 1 commit intodevelopfrom
Conversation
…yback
Implements a robust media selection strategy to improve Direct Play
reliability. The system now dynamically checks all available media streams
against device capabilities and prioritizes the most compatible format.
This ensures that if a direct-playable combination exists, it is
selected for playback, falling back to other streams or transcoding
only when necessary.
Key changes include:
- **Media Sorting:** `MovieMediaContainer` and `EpisodeMediaContainer` now
sort media combinations by device compatibility before selecting one.
This is driven by a new `isDirectPlaySupported` helper method that has
been centralized in `AbstractVideoContentInfo`.
- **Dynamic Capability Checking:**
- `AndroidHelper` now uses ExoPlayer's `AudioCapabilities` to
dynamically detect audio passthrough support, removing fragile,
hardcoded device and codec checks.
- `MediaCodecInfoUtil` has been refactored to cache codec support information,
improving performance and removing redundant lookups.
- **Robust Combination Generation:** Fixed a critical bug in
`MediaContainerAdaptor` where only the last media stream was being
considered. It now correctly generates all valid video/audio
combinations for a given media item.
- **Testing:**
- Added new unit tests in `MovieMediaContainerTest` and
`EpisodeMediaContainerTest` to verify the new media sorting logic.
- Updated existing tests to provide the new `AndroidHelper` mock
dependency, ensuring test stability.
- Corrected a test-related crash by making production code in
`EpisodeMediaContainer` more robust against unexpected data.
WalkthroughThis change extends the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 #491 +/- ##
=============================================
+ Coverage 47.87% 48.28% +0.41%
+ Complexity 445 442 -3
=============================================
Files 59 59
Lines 1878 1893 +15
Branches 252 255 +3
=============================================
+ Hits 899 914 +15
+ Misses 892 891 -1
- Partials 87 88 +1
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}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
serenity-app/src/main/java/us/nineworlds/serenity/core/util/AndroidHelper.java (1)
1-107: Consider converting to Kotlin.Per coding guidelines, the primary language is Kotlin and Java files should be converted to Kotlin where feasible. This utility class is a good candidate for conversion.
Based on coding guidelines, **/*.java: Primary language is Kotlin; prefer converting Java to Kotlin where feasible.
emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/adapters/MediaContainerAdaptor.kt (3)
33-55: Category construction logic OK; consider naming clean‑upThe category list construction (including conditional unwatched/recent/on‑deck entries) is clear and should behave as expected for
series == false.Two minor polish points you might consider (non‑blocking):
createAllCatagory/createSeaonsListcontain typos in the identifiers, which can be confusing when reading or searching.- The local variable name
allCategoryis reused in helpers that aren’t actually “all” (Unwatched, Recently Added, OnDeck); a more generic name (categoryDir) would be clearer.These are purely readability/API‑surface concerns and can be deferred.
Also applies to: 57-83
85-124: Series leaf/view counts – double‑check semanticsThe series builder is generally solid (titles, ratings, artwork paths, and user data wiring all make sense). One thing to verify:
totalItemCountis computed asunplayedItemCount + playCount. IfplayCountrepresents “number of plays” rather than “number of played items”, this could overcount leaves and inflateleafCount.If Emby’s
playCounthere is indeed “played items count” this is fine; otherwise it might be worth switching to a more direct count field from the API (if available).
126-128: Season list stub may lead to empty UI
createSeaonsListcurrently just returns an emptyMediaContainer. If this is already wired into any UI flow, it will silently show no seasons.If this is intentionally a placeholder, consider:
- Adding a
TODOcomment, or- Implementing a minimal season mapping similar to
createSeriesList/createVideoList.Also, you may want to fix the spelling in the API name when you flesh this out (
createSeasonsList).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/adapters/MediaContainerAdaptor.kt(1 hunks)serenity-app/src/main/java/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfo.kt(2 hunks)serenity-app/src/main/java/us/nineworlds/serenity/core/util/AndroidHelper.java(1 hunks)serenity-app/src/main/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainer.kt(2 hunks)serenity-app/src/main/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainer.kt(1 hunks)serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.kt(1 hunks)serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt(4 hunks)serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.kt(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{kt,java}
📄 CodeRabbit inference engine (AGENTS.md)
Run formatting before commit: ./gradlew spotlessApply
Files:
serenity-app/src/main/java/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfo.ktserenity-app/src/main/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainer.ktserenity-app/src/main/java/us/nineworlds/serenity/core/util/AndroidHelper.javaserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/main/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainer.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.ktemby-lib/src/main/kotlin/us/nineworlds/serenity/emby/adapters/MediaContainerAdaptor.kt
**/*.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*.kt: Follow standard Kotlin naming conventions (camelCase for variables/functions, PascalCase for classes)
Use Toothpick for dependency injection with @Inject annotation and provider-based presenter injection for testability
Use Kotlin Coroutines for asynchronous work
Use Retrofit + OkHttp for network calls
Use Glide for image loading
Prefer immutable data structures where possible
Prefer immutability and read-only properties in data models
Files:
serenity-app/src/main/java/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfo.ktserenity-app/src/main/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainer.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/main/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainer.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.ktemby-lib/src/main/kotlin/us/nineworlds/serenity/emby/adapters/MediaContainerAdaptor.kt
**/*.java
📄 CodeRabbit inference engine (AGENTS.md)
Primary language is Kotlin; prefer converting Java to Kotlin where feasible
Files:
serenity-app/src/main/java/us/nineworlds/serenity/core/util/AndroidHelper.java
**/*Test.kt
📄 CodeRabbit inference engine (AGENTS.md)
**/*Test.kt: Tests that require DI should inherit from InjectingTest
Unit tests required for all new features and bug fixes; use MockK for mocking with mockk(relaxed = true) for simple stubbing
Define mocks as properties in test classes and instantiate with mockk()
Use @before for setup and @after for teardown; always call clearAllMocks() in @after
Static mocks (mockkStatic/mockkObject) should be set up and torn down in @BeforeClass/@afterclass as required
Use Robolectric for Android-framework dependent tests
When converting tests to Kotlin, replace AssertJ assertions with AssertK equivalents
Test classes should extend InjectingTest and override setUp() calling super.setUp() first; implement installTestModules() to bind test modules
Files:
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
🧠 Learnings (9)
📚 Learning: 2025-11-28T20:52:23.735Z
Learnt from: CR
Repo: NineWorlds/serenity-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T20:52:23.735Z
Learning: Applies to **/*.kt : Use Toothpick for dependency injection with Inject annotation and provider-based presenter injection for testability
Applied to files:
serenity-app/src/main/java/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfo.kt
📚 Learning: 2025-11-28T20:52:23.735Z
Learnt from: CR
Repo: NineWorlds/serenity-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T20:52:23.735Z
Learning: Applies to **/*Test.kt : Unit tests required for all new features and bug fixes; use MockK for mocking with mockk(relaxed = true) for simple stubbing
Applied to files:
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
📚 Learning: 2025-11-28T20:52:23.735Z
Learnt from: CR
Repo: NineWorlds/serenity-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T20:52:23.735Z
Learning: Applies to **/*Test.kt : Define mocks as properties in test classes and instantiate with mockk()
Applied to files:
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
📚 Learning: 2025-11-28T20:52:23.735Z
Learnt from: CR
Repo: NineWorlds/serenity-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T20:52:23.735Z
Learning: Applies to **/*Test.kt : Test classes should extend InjectingTest and override setUp() calling super.setUp() first; implement installTestModules() to bind test modules
Applied to files:
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
📚 Learning: 2025-11-28T20:52:23.735Z
Learnt from: CR
Repo: NineWorlds/serenity-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T20:52:23.735Z
Learning: Applies to **/*Test.kt : Tests that require DI should inherit from InjectingTest
Applied to files:
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
📚 Learning: 2025-11-28T20:52:23.735Z
Learnt from: CR
Repo: NineWorlds/serenity-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T20:52:23.735Z
Learning: Applies to **/*Test.kt : Use Robolectric for Android-framework dependent tests
Applied to files:
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
📚 Learning: 2025-11-28T20:52:23.735Z
Learnt from: CR
Repo: NineWorlds/serenity-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T20:52:23.735Z
Learning: Applies to **/*Test.kt : When converting tests to Kotlin, replace AssertJ assertions with AssertK equivalents
Applied to files:
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
📚 Learning: 2025-11-28T20:52:23.735Z
Learnt from: CR
Repo: NineWorlds/serenity-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T20:52:23.735Z
Learning: Applies to **/*Test.kt : Static mocks (mockkStatic/mockkObject) should be set up and torn down in BeforeClass/AfterClass as required
Applied to files:
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
📚 Learning: 2025-11-28T20:52:23.735Z
Learnt from: CR
Repo: NineWorlds/serenity-android PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-28T20:52:23.735Z
Learning: Applies to **/*Test.kt : Use Before for setup and After for teardown; always call clearAllMocks() in After
Applied to files:
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.ktserenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
🧬 Code graph analysis (3)
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.kt (3)
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt (1)
bind(204-210)serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.kt (1)
bind(169-175)serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractMediaContainerTest.kt (1)
bind(62-67)
serenity-app/src/main/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainer.kt (1)
serenity-app/src/main/java/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfo.kt (1)
isDirectPlaySupported(181-193)
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt (2)
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.kt (1)
bind(169-175)serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.kt (1)
bind(169-175)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (16)
serenity-app/src/main/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainer.kt (1)
48-49: LGTM! Smart media selection strategy implemented correctly.The sorting logic properly prioritizes direct-playable media combinations by checking device compatibility before selection. The use of
sortedByDescendingwith theisDirectPlaySupportedpredicate ensures that the most compatible media is chosen first, with graceful fallback to other media if no direct-playable option exists.The implementation follows Kotlin best practices:
- Maintains immutability by creating a new sorted list
- Uses safe call operators for null safety
- Leverages stable sorting to preserve original order when direct-play support is equal
serenity-app/src/main/java/us/nineworlds/serenity/core/util/AndroidHelper.java (1)
68-70: Verify impact of disabling tunneling universally.The method now unconditionally returns
false, which disables audio/video tunneling for all devices. Ensure this change aligns with the expected Direct Play behavior and doesn't negatively impact playback performance on devices that previously benefited from tunneling.serenity-app/src/main/java/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfo.kt (2)
20-26: DI initialization follows Toothpick pattern correctly.The dependency injection setup properly uses the
@Injectannotation and initializesandroidHelpervia Toothpick in the init block.Based on learnings, Toothpick is used for dependency injection with the
@Injectannotation.
181-193: No issues found. Themedia.audioCodecandmedia.videoCodecproperties store plain codec names (e.g., "aac", "h264") without MIME type prefixes. ThefindCorrectAudioMimeType()andfindCorrectVideoMimeType()methods correctly handle the prepended "audio/" and "video/" prefixes by usingsubstringAfter()to safely extract the codec portion before applying their transformations. The implementation is safe and will not produce malformed MIME types.Likely an incorrect or invalid review comment.
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/AbstractVideoContentInfoTest.kt (2)
29-64: Test structure follows coding guidelines correctly.The test properly:
- Extends
InjectingTest- Uses
mockk(relaxed = true)for mocks- Calls
super.setUp()first- Uses
clearAllMocks()in@After- Manages static mocks in companion object with
@BeforeClass/@AfterClassBased on coding guidelines, tests should extend InjectingTest and follow the established patterns for setup/teardown.
169-175: AndroidHelper binding aligns with DI pattern.The TestModule correctly binds the mock AndroidHelper instance, enabling dependency injection for tests.
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt (3)
85-89: Comprehensive codec support mocking.The test properly mocks both
MediaCodecInfoUtilstatic methods andAndroidHelperinstance methods to simulate codec support checks for direct play scenarios.
166-202: Direct play sorting test validates new behavior.The test effectively verifies that media with supported codecs (ac3/h264) is selected over unsupported media (flac/hevc) through the sorting mechanism.
208-208: AndroidHelper DI binding enables test mocking.Binding
AndroidHelperin the TestModule allows tests to control audio passthrough behavior.serenity-app/src/main/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainer.kt (2)
17-17: More robust null/empty checking.Using
isNullOrEmpty()is more idiomatic than a simple null check and prevents potential issues with empty strings.
63-65: Sorting prioritizes direct-play-compatible media.The implementation correctly sorts media by direct play support (descending) and selects the first item. This ensures that if any media is direct-play-compatible, it will be chosen.
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.kt (3)
78-80: Codec support mocking enables direct play testing.The test properly sets up mocks for
MediaCodecInfoUtilandAndroidHelperto validate codec support checks during media selection.
131-167: Direct play sorting test validates selection logic.The test effectively verifies that the media selection prioritizes supported codecs (ac3/h264) over unsupported ones (flac/hevc), matching the behavior in
EpisodeMediaContainerTest.
173-173: AndroidHelper DI binding consistent across tests.The TestModule binding matches the pattern used in other test files, enabling consistent mocking of Android capabilities.
emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/adapters/MediaContainerAdaptor.kt (2)
14-16: Time tick conversion helper looks goodUsing a single
TICKS_PER_MILLISECONDconstant andconvertTicksToMillisecondskeeps the tick math centralized and avoids magic numbers. No issues here.Also applies to: 221-221
18-31: Main menu mapping is straightforward and correctThe
createMainMenuimplementation is a simple projection fromItemtoDirectorywith no obvious edge‑case pitfalls. This looks fine as is.
| fun createVideoList(videos: List<Item>): IMediaContainer { | ||
| val mediaContainer = MediaContainer() | ||
| val serenityVideos = ArrayList<Video>() | ||
| mediaContainer.size = videos.size | ||
|
|
||
| val items = videos.filter { item -> item.type != "Folder" } | ||
| .filterNot { item -> item.name == "TBA" } | ||
|
|
||
| for (item in items) { | ||
| val video = Video() | ||
|
|
||
| val sortEpisode = item.episodeNumber?.toInt() ?: 0 | ||
|
|
||
| video.type = item.type | ||
| video.titleSort = sortEpisode.toString().padStart(3, '0') | ||
| video.title = item.name | ||
| video.key = item.id | ||
| video.parentKey = item.parentId | ||
| video.contentRating = item.officialRating | ||
| video.summary = item.oveview | ||
| video.rating = item.communityRating ?: 0.00 | ||
| video.season = item.parentIndexNumber | ||
| video.seriesName = item.seriesName | ||
|
|
||
| if (item.type != null && item.type == "Episode") { | ||
| video.backgroundImageKey = "/emby/Items/${item.parentId}/Images/Backdrop" | ||
| video.parentThumbNailImageKey = "/emby/Items/${item.parentId}/Images/Primary" | ||
| } else { | ||
| video.backgroundImageKey = "/emby/Items/${item.id}/Images/Backdrop" | ||
| } | ||
| video.thumbNailImageKey = "/emby/Items/${item.id}/Images/Primary" | ||
| video.viewCount = item.userData?.playCount?.toInt() ?: 0 | ||
| val offset = convertTicksToMilliseconds(item.userData?.playbackPositionTicks ?: 0) | ||
| video.viewOffset = offset | ||
| video.episode = item.episodeNumber | ||
|
|
||
| val container = if (item.container != null && item.container.contains(",")) { | ||
| item.container.substringBefore(",") | ||
| } else { | ||
| item.container | ||
| } | ||
| video.directPlayUrl = | ||
| "emby/Videos/${item.mediaSources?.get(0)?.id ?: item.id}/stream.$container?static=true" | ||
|
|
||
| if (item.runTimeTicks != null) { | ||
| val milliseconds = convertTicksToMilliseconds(item.runTimeTicks) | ||
| video.duration = milliseconds | ||
| } | ||
|
|
||
| video.medias = createPlayableLists(item) | ||
|
|
||
| serenityVideos.add(video) | ||
| } | ||
| mediaContainer.videos = serenityVideos.sortedBy { item -> item.titleSort }.toList() | ||
| return mediaContainer | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, verify the file exists and examine the code at the specified lines
find . -type f -name "MediaContainerAdaptor.kt" -o -name "*MediaContainer*" | head -20Repository: NineWorlds/serenity-android
Length of output: 1961
🏁 Script executed:
# List files in the emby-lib directory to understand the structure
git ls-files emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/ | head -30Repository: NineWorlds/serenity-android
Length of output: 1116
🏁 Script executed:
# Read the actual MediaContainerAdaptor.kt file to verify the code
cat -n emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/adapters/MediaContainerAdaptor.ktRepository: NineWorlds/serenity-android
Length of output: 9398
🏁 Script executed:
# Find and examine the Item model
find . -name "itemsmodel.kt" -o -name "*Item.kt" | grep -E "(emby|model)" | head -10Repository: NineWorlds/serenity-android
Length of output: 242
🏁 Script executed:
# Read the Item model definition
cat -n emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/server/model/itemsmodel.ktRepository: NineWorlds/serenity-android
Length of output: 5302
🏁 Script executed:
# Check if there are any tests for MediaContainerAdaptor
find . -name "*MediaContainerAdaptor*" -type fRepository: NineWorlds/serenity-android
Length of output: 264
🏁 Script executed:
# Check for tests of createVideoList
find . -type f -name "*.kt" | xargs grep -l "createVideoList" 2>/dev/nullRepository: NineWorlds/serenity-android
Length of output: 442
🏁 Script executed:
# Check the JellyfinMediaContainerAdaptor for comparison
cat -n jellyfin-lib/src/main/kotlin/us/nineworlds/serenity/jellyfin/adapters/JellyfinMediaContainerAdaptor.kt | head -220Repository: NineWorlds/serenity-android
Length of output: 8083
Fix three robustness issues in video list creation
The code has three correctness concerns that should be addressed:
-
IndexOutOfBoundsExceptionon emptymediaSources
item.mediaSources?.get(0)?.idthrows ifmediaSourcesis a non-null but empty list. UsefirstOrNull()instead. -
mediaContainer.sizemismatch
Line 133 setsmediaContainer.size = videos.sizebefore filtering. Lines 135–136 then filter out folders and "TBA" items, causing the reported size to exceed the actual video count. Set size toitems.sizeafter filtering. -
Null
containerproducesstream.null
Whenitem.containeris null, the URL becomesstream.null. Use a safe approach to append the extension only when container exists:val container = item.container?.let { raw -> if (raw.contains(",")) raw.substringBefore(",") else raw } val mediaSourceId = item.mediaSources?.firstOrNull()?.id ?: item.id val extension = container?.let { ".$it" } ?: "" video.directPlayUrl = "emby/Videos/$mediaSourceId/stream$extension?static=true"
🤖 Prompt for AI Agents
In
emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/adapters/MediaContainerAdaptor.kt
around lines 130 to 185, address three robustness issues: (1) replace
mediaSources?.get(0)?.id with a safe lookup using firstOrNull() so an empty
non-null list doesn't throw IndexOutOfBoundsException; (2) set
mediaContainer.size after filtering (use items.size) instead of using the
original videos.size so the reported size matches the actual list; (3) avoid
producing "stream.null" by handling a null container safely — extract the
container extension only when non-null (and handle comma-separated values),
build the mediaSourceId via firstOrNull() fallback to item.id, and append the
extension only if present when composing directPlayUrl.
| fun createPlayableLists(item: Item): List<Media> { | ||
| val medias = mutableListOf<Media>() | ||
| if (item.mediaStreams.isNullOrEmpty()) { | ||
| return medias | ||
| } | ||
|
|
||
| val videoStreams = item.mediaStreams.filter { it.type == "Video" } | ||
| val audioStreams = item.mediaStreams.filter { it.type == "Audio" } | ||
|
|
||
| // If there's no video or no audio, no valid combination can be made. | ||
| if (videoStreams.isEmpty() || audioStreams.isEmpty()) { | ||
| return medias | ||
| } | ||
|
|
||
| videoStreams.forEach { videoStream -> | ||
| audioStreams.forEach { audioStream -> | ||
| val media = Media() // Create a new object for each combination | ||
| media.container = item.container | ||
|
|
||
| // From video stream | ||
| media.aspectRatio = videoStream.aspectRatio | ||
| media.videoCodec = videoStream.codec | ||
|
|
||
| // From audio stream | ||
| media.audioCodec = audioStream.codec | ||
| media.audioChannels = audioStream.channels | ||
|
|
||
| medias.add(media) | ||
| } | ||
| } | ||
| video.medias = medias.toList() | ||
| } | ||
|
|
||
| serenityVideos.add(video) | ||
| return medias | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "MediaContainerAdaptor.kt" --type fRepository: NineWorlds/serenity-android
Length of output: 260
🏁 Script executed:
find . -name "MediaContainerAdaptor.kt" -type f -exec wc -l {} \;Repository: NineWorlds/serenity-android
Length of output: 163
🏁 Script executed:
rg "class MediaContainerAdaptor" -A 5Repository: NineWorlds/serenity-android
Length of output: 707
🏁 Script executed:
find . -path "*emby*" -name "*Adaptor*.kt" -type fRepository: NineWorlds/serenity-android
Length of output: 159
🏁 Script executed:
cat -n emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/adapters/MediaContainerAdaptor.ktRepository: NineWorlds/serenity-android
Length of output: 9398
🏁 Script executed:
find . -name "*.kt" -type f -exec grep -l "class Media" {} \;Repository: NineWorlds/serenity-android
Length of output: 496
🏁 Script executed:
rg "Media\(" -A 3 | head -50Repository: NineWorlds/serenity-android
Length of output: 1165
🏁 Script executed:
rg "\.container" --type kt -B 2 -A 2 | head -100Repository: NineWorlds/serenity-android
Length of output: 97
🏁 Script executed:
find . -path "*emby-lib*" -name "*.kt" -type f | grep -i mediaRepository: NineWorlds/serenity-android
Length of output: 159
🏁 Script executed:
rg "\.container\s*[=:]|container.*=" --type-list | head -5Repository: NineWorlds/serenity-android
Length of output: 460
🏁 Script executed:
rg "\.container" -B 2 -A 2 | grep -E "(container|direct|play|capability)" | head -60Repository: NineWorlds/serenity-android
Length of output: 5609
🏁 Script executed:
rg "isDirectPlaySupportedForContainer" -B 2 -A 10Repository: NineWorlds/serenity-android
Length of output: 3664
Apply container normalization in createPlayableLists to match createVideoList handling
The code in createVideoList normalizes comma-separated container strings before use (lines 166-170), but createPlayableLists assigns the raw item.container directly to media.container (line 204). This inconsistency should be corrected to ensure predictable container values downstream.
Apply this normalization in createPlayableLists:
media.container = item.container?.let { raw ->
if (raw.contains(",")) raw.substringBefore(",") else raw
}This keeps the container handling consistent across both methods and prevents potential issues if future capability checks or direct-play logic depend on normalized container values.
🤖 Prompt for AI Agents
In
emby-lib/src/main/kotlin/us/nineworlds/serenity/emby/adapters/MediaContainerAdaptor.kt
around lines 187 to 219, createPlayableLists assigns item.container directly to
media.container, causing inconsistent container values vs createVideoList; fix
by normalizing item.container the same way as createVideoList: if item.container
is null leave media.container null, otherwise if it contains a comma set
media.container to the substring before the first comma, else set it to the
original string (use safe null handling to avoid NPEs).
| override fun setUp() { | ||
| super.setUp() | ||
| every { mockSerenityClient.baseURL() } returns "http://localhost:8096/" | ||
| mockkObject(MediaCodecInfoUtil) |
There was a problem hiding this comment.
Add cleanup for mockkObject to prevent test pollution.
mockkObject(MediaCodecInfoUtil) is initialized in @Before but there's no corresponding unmockkObject in @After. This can cause test pollution if MediaCodecInfoUtil is used by other tests.
Based on coding guidelines, static mocks should be properly torn down.
Apply this diff:
@After
fun tearDown() {
+ unmockkObject(MediaCodecInfoUtil)
clearAllMocks()
}📝 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.
| mockkObject(MediaCodecInfoUtil) | |
| @After | |
| fun tearDown() { | |
| unmockkObject(MediaCodecInfoUtil) | |
| clearAllMocks() | |
| } |
🤖 Prompt for AI Agents
In
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/EpisodeMediaContainerTest.kt
around line 43, the test uses mockkObject(MediaCodecInfoUtil) in setup but lacks
teardown, causing static mock leakage; add an @After method that calls
unmockkObject(MediaCodecInfoUtil) (or unmockkAll() if appropriate) to restore
original behavior and prevent test pollution, ensuring the static mock is
properly removed after each test.
| override fun setUp() { | ||
| super.setUp() | ||
| every { mockSerenityClient.baseURL() } returns "http://localhost:8096/" | ||
| mockkObject(MediaCodecInfoUtil) |
There was a problem hiding this comment.
Add cleanup for mockkObject to prevent test pollution.
mockkObject(MediaCodecInfoUtil) is initialized in @Before but there's no corresponding unmockkObject in @After. This can cause test pollution if MediaCodecInfoUtil is used by other tests.
Based on coding guidelines, static mocks should be properly torn down.
Apply this diff:
@After
fun tearDown() {
+ unmockkObject(MediaCodecInfoUtil)
clearAllMocks()
}📝 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.
| mockkObject(MediaCodecInfoUtil) | |
| @After | |
| fun tearDown() { | |
| unmockkObject(MediaCodecInfoUtil) | |
| clearAllMocks() | |
| } |
🤖 Prompt for AI Agents
In
serenity-app/src/test/kotlin/us/nineworlds/serenity/core/model/impl/MovieMediaContainerTest.kt
around line 42, the test sets up a static mock with
mockkObject(MediaCodecInfoUtil) in @Before but never tears it down; add an
@After teardown that calls unmockkObject(MediaCodecInfoUtil) (or unmockkAll() if
you prefer clearing all static mocks) to ensure the static mock is removed after
each test and prevent cross-test pollution.
1 participant
…yback
Implements a robust media selection strategy to improve Direct Play reliability. The system now dynamically checks all available media streams against device capabilities and prioritizes the most compatible format.
This ensures that if a direct-playable combination exists, it is selected for playback, falling back to other streams or transcoding only when necessary.
Key changes include:
Media Sorting:
MovieMediaContainerandEpisodeMediaContainernow sort media combinations by device compatibility before selecting one. This is driven by a newisDirectPlaySupportedhelper method that has been centralized inAbstractVideoContentInfo.Dynamic Capability Checking:
AndroidHelpernow uses ExoPlayer'sAudioCapabilitiesto dynamically detect audio passthrough support, removing fragile, hardcoded device and codec checks.MediaCodecInfoUtilhas been refactored to cache codec support information, improving performance and removing redundant lookups.Robust Combination Generation: Fixed a critical bug in
MediaContainerAdaptorwhere only the last media stream was being considered. It now correctly generates all valid video/audio combinations for a given media item.Testing:
MovieMediaContainerTestandEpisodeMediaContainerTestto verify the new media sorting logic.AndroidHelpermock dependency, ensuring test stability.EpisodeMediaContainermore robust against unexpected data.Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.