Skip to content

Refactor: Extract HEVC and AV1 codec modules#5646

Open
johnpc wants to merge 2 commits into
jellyfin:masterfrom
johnpc:refactor/device-profile-hevc-av1-modules
Open

Refactor: Extract HEVC and AV1 codec modules#5646
johnpc wants to merge 2 commits into
jellyfin:masterfrom
johnpc:refactor/device-profile-hevc-av1-modules

Conversation

@johnpc

@johnpc johnpc commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Changes

This follows up on #5604 by extracting the HEVC and AV1 codec detection logic out of MediaCodecCapabilitiesTest into their own modules. After this change, MediaCodecCapabilitiesTest is just a thin delegation facade with no logic of its own.

Code assistance

Claude Code was used as an assistant during implementation, code review, and test authoring.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

class Av1CodecCapabilities(
private val query: MediaCodecQuery,
private val sdkInt: Int = Build.VERSION.SDK_INT,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need to set the sdkInt as a parameter? It's a constant (see the default value you're assigning).

@johnpc johnpc Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There's branching logic in Av1CodecCapabilities based on the value of sdkInt, which I wanted to stress in the unit tests. Mocking Build.VERSION.SDK_INT within the test file itself might be possible but there's not a pattern for it in the code, so I'd need to invent one (Robolectric? Reflection? Both feel more controversial than exposing the defaulted param)

Alternatively I might be able to remove the branching completely if it's truly dead code (and therefore have no need to test it), but my goal was to move and test the existing logic, not delete it.

What do you think? My preference personally would be to merge as-is, but happy to defer to your opinion on it.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I believe you can just mock the version_sdk_int with mockk for that

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think Build.VERSION.SDK_INT can't be directly mocked. I tried it and the tests failed with IllegalAccessException.

I've added a commit to remove the field by adding a small layer of indirection via a new DeviceSdk internal object that I can mock. Let me know if that is better

@johnpc johnpc requested a review from nielsvanvelzen June 13, 2026 14:52
…ject

Remove the sdkInt parameter from Av1CodecCapabilities and
HevcCodecCapabilities. Instead, read Build.VERSION.SDK_INT via an
internal DeviceSdk object that tests can mock with mockkObject.
thor2002ro added a commit to thor2002ro/jellyfin-androidtv that referenced this pull request Jun 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants