Repository navigation
fix(network): support HLS init segments (EXT-X-MAP) and discontinuities (#177) - #225
Conversation
BGLuis
left a comment
There was a problem hiding this comment.
The parser side is clean and matches the issue's step 1 closely: EXT-X-MAP attaches to every following segment, EXT-X-DISCONTINUITY marks exactly the next one via mem::take, and the new parser test asserts both, including the map without BYTERANGE. The init-before-media path mirrors the DASH precedent (dash/stream.rs init_segments cache), and the two httpmock stream tests cover acceptance criteria 2 and the single-variant case. A throwaway probe also confirmed that init.mp4 + p0..p2.m4s from the same ffmpeg fMP4 output decodes cleanly end to end (149 frames, no warnings), so the plain fMP4 VOD case this PR targets does work.
Main points
EXT-X-DISCONTINUITYon a segment that has a map makesensure_bufferemit the init section forever and never fetch the segment (reproduced; inline onstream.rs:204). No stream test covers a discontinuity, which is why it passed.- The issue's step 3 and its goal ("does not hand the demuxer a spliced, inconsistent byte stream") are not met: an ABR switch appends the new variant's
ftyp/moovinto the same byte stream, and libavformat's mov demuxer skips any secondmoov(Found duplicated MOOV Atom. Skipped it,libavformat/mov.c:1603in the bundled 8.1.2). Reproduced: splicing a 640x480 variant after a 320x240 one decodes every frame at 320 wide with h264 errors (inline onstream.rs:148). Either implement the demux restart the issue asks for, or changeCloses #177toRefs #177and track the rest.
Smaller
BYTERANGE="0@…"underflowsoffset + length - 1and panics in debug (reproduced); same pattern already exists infetch_segmentand DASH.- The integration test only asserts
total > 20_000bytes, so it does not check criterion 3's "opens and plays to the end". - New comments, doc comments, and test messages are in Portuguese;
CLAUDE.md/CONTRIBUTING.mdask for English in new and edited comments.
Dependencies: no package version changed (dependency_diff.py, no Cargo.toml/Cargo.lock change; httpmock 0.8.3 was already a dev-dependency). The added reqwest 0.13 calls (get, header, send, bytes) are the same ones fetch_segment already uses, none deprecated. The FFmpeg behavior above was checked against the source of the version ffmpeg-android-maker pins (8.1.2) and with a local FFmpeg n9.0.2.
Checked: every changed line against CLAUDE.md/CONTRIBUTING.md (layering: logic stays in host-testable protocols; language rule), issue #177's four acceptance criteria, and the DASH init-segment precedent. Ran cargo fmt --check, cargo clippy -p protocols -p media-logic --all-targets --all-features -- -D warnings (clean), cargo test -p protocols hls (10 pass), throwaway probes for the discontinuity loop and the zero-length byte range (both reproduce), and an ffmpeg splice probe for the ABR switch. Not run: ./scripts/test-network-protocols.sh (the Docker integration suite), and nothing on the headset.
- Track init_emitted_for_segment in HlsStreamSource to avoid infinite init re-emission on discontinuities. - Add checked format_byte_range_header to prevent underflow/overflow on len == 0 or hostile inputs. - Suppress mid-stream fMP4 variant switching to prevent moov atom corruption in FFmpeg demuxer. - Enhance integration test to assert 'ftyp' header and exact payload size. - Translate comments and test messages into English per project rules. Refs #177
PR Review Fixes AppliedAll review points have been addressed in commit
|
Description
Resolves review findings for #177 by adding RFC 8216 support for fragmented MP4 (fMP4/CMAF) initialization segments (
#EXT-X-MAP) and stream discontinuities (#EXT-X-DISCONTINUITY):• Added
HlsMapstruct withuriand optional byte range(offset, length)torust/protocols/src/hls/playlist.rs.• Added
map: Option<HlsMap>anddiscontinuity: booltoHlsSegment.• Implemented parser support for
#EXT-X-MAP:URI="...",BYTERANGE="..."and#EXT-X-DISCONTINUITYattributes. Discards invalid zero-length byte ranges.• Added checked
format_byte_range_header()inrust/protocols/src/hls/segment.rspreventing underflow/overflow onlen == 0or boundary additions.• Added initialization caching and sequencing logic to
HlsStreamSourceinrust/protocols/src/hls/stream.rs:ftyp/moov) before media chunks.init_emitted_for_segmentto prevent infinite init re-emission on discontinuities.test_hls_stream_source_discontinuity_emits_init_once_and_reads_all_media_segmentsvalidating that discontinuity streams terminate and deliver media segments.• Scope note: FFmpeg's
movdemuxer skips mid-stream duplicatedmoovatoms. Mid-stream ABR switching for fMP4 requires demuxer restart across C ABI (matching current DASH limitation). Mid-stream ABR reload is suppressed for fMP4, maintaining the selected variant across playback.• Updated
scripts/test-network-protocols.shto generate anhls_fmp4fixture and exportVRPLAYER_TEST_HLS_FMP4_SIZE.• Enhanced end-to-end integration test
hls_integration.rsto verify theftypbox header and exact concatenated payload size against active Nginx container.• Translated all new and edited comments, docstrings, and test assertion messages to English.
Refs #177
Refs #170
Type of change
[✓] 🐛 Bug fix (non-breaking change which fixes an issue)
[ ] ✨ New feature (non-breaking change which adds functionality)
[ ] 🛠️ Technical refactor / Code improvement
[ ] 📝 Documentation update
[ ] 🚀 Performance optimization
[ ] 💥 Breaking change (fix or feature that would cause existing functionality to not work as expected)
Affected layers
[ ] 🟣 Kotlin (
app/) — app shell, UI panels, credentials, history, i18n[ ] 🔵 C++ (
native/) — OpenXR session, Vulkan/GLES render loop, controller input[✓] 🦀 Rust (
rust/) — demux, decode, audio, network protocols[✓] 🔧 Build / CI / scripts
[ ] 📚 Docs only
How has this been tested?
[✓] Unit tests (
cargo test):hls::playlist::testsandhls::stream::testspassed (13 passed, including fMP4 parsing, zero-length byte range rejection, init segment emission, and discontinuity resolution withhttpmock).[✓] Integration tests (
./scripts/test-network-protocols.sh): Verifiedhls_fmp4_reads_init_and_all_segments_from_real_serveragainst live Nginx container withftypheader validation and exact byte payload matching.[✓] Rust linter (
cargo clippy -p protocols -p media-logic --all-targets --all-features -- -D warnings) and formatter (cargo fmt --check) passed with 0 warnings/errors.Checklist:
[✓] My code follows the project's style guidelines (
ktlint,rustfmt,clippy -D warnings).[✓] I have performed a self-review of my own code.
[✓] I have commented my code, particularly in hard-to-understand areas.
[✓] I have added new tests that prove my fix is effective or that my feature works.
[ ] The CI checks are passing for this PR.
If applicable
[✓] Kotlin never calls Rust directly — the Kotlin → JNI → C++ → C ABI → Rust path is preserved (ADR-002).
[ ] I changed the screen/stereo mode enum and kept it in sync across all four places.
[ ] I added or changed user-facing strings and updated all supported locales.
[✓] Pure logic I added lives in
protocols(host-testable).[✓] No credential is stored or logged in plain text.