Skip to content

Fix embedded subtitle discovery and selection - #70

Open
enikesha wants to merge 24 commits into
rovkinmax:masterfrom
enikesha:fix/embedded-subtitles
Open

Fix embedded subtitle discovery and selection#70
enikesha wants to merge 24 commits into
rovkinmax:masterfrom
enikesha:fix/embedded-subtitles

Conversation

@enikesha

@enikesha enikesha commented Aug 27, 2026 •
edited
Loading

Copy link
Copy Markdown

Changes

  • Show player-discovered embedded HLS subtitles in the picker, using API subtitles as fallbacks when the manifest exposes none.
  • Display localized language names, mark forced tracks as partial, and keep same-language variants distinct.
  • Treat KinoPub's ai subtitle code as AI-generated content and display the localized Созданные ИИ label.
  • Preserve non-file source labels or raw uppercase tokens for other unknown language codes; use positional names only when the source supplies no identifying metadata.
  • Preserve subtitle selection and saved preferences across track reordering, quality changes, player restarts, and stream switches.
  • Carry typed StreamSource container metadata through playback instead of guessing HLS from URL text.
  • Use Media3 1.11's default media-source pipeline for HLS so side-loaded subtitle text is parsed into cues while retaining chunkless preparation and the custom load-error policy.
  • Adapt player mocks, callback tests, device/resource tests, and E2E coverage to the index-free subtitle identity and updated upstream interfaces.

Checks

  • ./gradlew :app:testDevDebugUnitTest (836 tests passed)
  • ./gradlew :app:testProdDebugUnitTest --tests 'com.kino.puber.ui.feature.player.*'
  • ./gradlew :player-test-fixtures:test
  • ./gradlew :app:compileInstrumentationDebugAndroidTestKotlin
  • ./gradlew :app:detektAll
  • ./gradlew :app:assembleProdDebug
  • tools/test-resolve-android-test-apk-pair
  • Android TV API 36: all 9 PlaybackControllerNetworkTracksTest tests passed.
  • Android TV API 36: PlayerScreenE2ETest movie screen remote-controls/panels/recreation test passed.

@enikesha enikesha changed the title Fix embedded subtitle discovery and selection Embedded subtitle discovery and selection Aug 27, 2026
@enikesha
enikesha force-pushed the fix/embedded-subtitles branch from 0e97154 to 3d2e5bf Compare August 29, 2026 08:42
@enikesha enikesha changed the title Embedded subtitle discovery and selection Discover and select embedded subtitle tracks Aug 29, 2026
@enikesha enikesha changed the title Discover and select embedded subtitle tracks Fix embedded subtitle discovery and selection Aug 29, 2026
@enikesha
enikesha marked this pull request as ready for review August 30, 2026 12:31
@rovkinmax

rovkinmax commented Aug 31, 2026

Copy link
Copy Markdown
Owner

пока что не смотрю PR, я готовлю инфру для инструментейшн тестов, чтобы можно независимо от реального бэка тестировать все сценарии в приложении. я отпишусь сюда дополнительно о том, что всё готово - надо будет покрыть всё тестами. щас ток юнит есть.

@enikesha
enikesha force-pushed the fix/embedded-subtitles branch from 0063b3b to 2f46779 Compare August 31, 2026 20:48
@rovkinmax

rovkinmax commented Sep 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

@enikesha я добавил моковые тесты для плеера, всё в мастере

enikesha and others added 22 commits September 2, 2026 13:27
@enikesha
Fix embedded subtitle discovery and merging
8c1a818
@enikesha
Handle same-language subtitle variants
105d59e
@enikesha
Fix HLS subtitle identity mapping
0ea108e
@enikesha
Use compact partial subtitle label
18fbef5
@enikesha @claude
Restrict HLS detection to the stream path
c686137 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Disambiguate same-language subtitle labels
333eab9 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Harden player track preference restore
f45a7eb 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Drop unreachable subtitle code paths
5fd63ed 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Side-load API subtitles on HLS streams
08f7e15 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Select subtitle tracks by merge-stable group id
64524da 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Present subtitles by language name and group variants
6e636a0 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Drop side-loaded subtitles the manifest already covers
1a9e70c 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Hide side-loaded subtitles only when the manifest covers them
e509096 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Treat the HLS manifest as the subtitle list when it has one
b449cf2 
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Remove dead subtitle track state and drive-by changes
a43d54a 
SubtitleTrackUIState.index lost its last production reader when the
positional text-track lookup was deleted, so drop the field and the
re-indexing pass it forced on every merge. Also fold the duplicated
subtitle file-extension regex into one, fold the tracksRestored flag
into resetTrackRestoreState, drop the unconsumed `selected` semantics
from SettingsPanelItem, and mark the deliberate SingleSampleMediaSource
deprecation instead of leaving a new build warning.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Cut redundant subtitle tests and the forced-URL guess
0495a3a 
Twelve tests either restated another test's assertion or asserted that a
mapper does nothing. Labelling cases move to SubtitleLabelerTest, which
owns them; the merger keeps only the cases where merging itself decides
the outcome. select_resolvesSideLoadedRow_byStableKey never reached the
stable-key rule (its group id matched first) - reshaped so that rule has
coverage for the first time.

SubtitleLink.forcedState guessed "forced" from a token in the subtitle
URL. It was written when the model had no forced flag; the flag was
added later and the comment rewritten to claim older responses omit it,
which nothing in the repo supports. Drop the guess and read the API
field directly.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Use one construction style in the subtitle tests
00710f6 
The track builders were called both as a single positional line and as a
five-line named block, for the same helper in the same file. Fold every
call that fits the line limit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Carry the stream container instead of guessing it from the URL
8d84a95 
selectStreamUrl picked url.hls4 / url.hls / url.http and then flattened
the choice to a String, so PlaybackController pattern-matched the URL
text to recover which branch had been taken. Return the container along
with the url and thread it through prepare/switchStream.

Deletes isHlsStreamUrl, its two regex constants, and HlsStreamUrlTest,
whose five cases all defended a guess that no longer happens.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha @claude
Drop a selector rule that cannot fire
3b6acd9 
The stable-key rule needed a track's format id to change while its label
stayed put. Merged source children are fixed at construction, so that
never happens; the test for it had to be contrived to reach the rule.
Also drop the resolver test that asserted matcher ordering rather than
any outcome a viewer can observe.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GEBLAtGvP5AHqHemWgLXAp
@enikesha
Harden embedded subtitle fallback and restore
004c965
@enikesha @claude
Drop the removed subtitle index from the focus test
b9d5c07 
The player focus test constructs SubtitleTrackUIState positionally, and
this branch removed the leading index field, so the instrumentation
androidTest source stopped compiling once it moved onto current master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQZbbPpsQNTRYLV15Lt5Bz
@enikesha
Adapt embedded subtitles to updated player fixtures
cfe1a06
@enikesha
enikesha force-pushed the fix/embedded-subtitles branch from 2f46779 to cfe1a06 Compare September 2, 2026 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@enikesha @rovkinmax