feat: add structured parsing sample data support#2
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds structured data parsing for MP4 sample table boxes, enabling programmatic access to detailed sample information. However, the implementation has several critical issues that prevent it from working as intended.
Key Changes:
- Added
StructuredDataenum and related structs to represent parsed MP4 box contents in typed format - Updated 8 box decoders (stsd, stts, ctts, stsc, stsz, stss, stco, co64) to return structured data
- Added new
samplesmodule with API for extracting track sample information - Added
mp4samplesbinary andtyped_sample_tablesexample to demonstrate the feature
Critical Issues Found:
- The
samples.rsmodule parses debug strings instead of using the actualBoxValue::Structureddata, making it fragile and defeating the purpose of structured parsing - Multiple functions return hardcoded placeholder values (track IDs, timescales, file offsets) instead of parsing real data
- The
StsdDecoderdoesn't parse version/flags and only processes one entry instead of all entries - Test coverage is incomplete - only 3 of 8 decoders are tested
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| src/registry.rs | Adds StructuredData types and updates 8 decoders to return structured data; StsdDecoder has incomplete parsing |
| src/samples.rs | New module for extracting track samples; critically flawed - parses debug strings instead of structured data and returns placeholder values |
| src/lib.rs | Exports new structured data types and sample extraction API |
| src/api.rs | Adds handling for BoxValue::Structured in decode_value function |
| src/bin/mp4dump.rs | Adds structured data formatting support |
| src/bin/mp4samples.rs | New binary for displaying sample info; uses placeholder values instead of real parsed data |
| examples/typed_sample_tables.rs | Example demonstrating structured parsing; has unused import |
| tests/registry_tests.rs | Tests for 3 decoders (stts, stsz, stsc); missing tests for ctts, stss, stco, co64, stsd |
| Cargo.toml | Declares new binaries (mp4info, mp4samples) and examples |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 11 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 11 changed files in this pull request and generated 19 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This pull request introduces structured data decoding for MP4 sample table boxes, making it possible to programmatically access detailed information from boxes like
stts,ctts,stsc,stsz,stss,stco,co64, andstsd. It also adds new binaries and examples, including a demonstration of how to analyze and parse sample tables using the new structured output. The changes improve both the API and internal decoders, enabling richer introspection and easier downstream processing of MP4 files.Structured data support for sample table boxes
StructuredDataenum and related structs (e.g.,SttsData,CttsData,StscData, etc.) to represent parsed box contents in a typed format. Decoders for sample table boxes now return this structured data instead of plain text. (src/registry.rs)StsdDecoder,SttsDecoder,CttsDecoder,StscDecoder,StszDecoder,StssDecoder,StcoDecoder,Co64Decoder) to parse their payloads into the corresponding structured types and return them viaBoxValue::Structured. (src/registry.rs) [1] [2] [3] [4] [5] [6] [7] [8]src/lib.rs,src/api.rs) [1] [2]Example and binary additions
mp4info,mp4samples) and examples (simple,boxes,typed_sample_tables) toCargo.tomlfor demonstrating and testing the structured decoding features.examples/typed_sample_tables.rs) that shows how to analyze sample table boxes and access their structured contents, including a demonstration of direct parsing using the registry.Minor improvements
src/api.rs)