Repository navigation
Handle legacy splice command length - #10
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates SCTE-35 parsing/encoding to correctly handle the legacy splice_command_length == 0xFFF sentinel by ignoring the declared length for known commands, while also cleaning up the public API by removing derived wire-length fields from SpliceInfoSection and computing those lengths during encoding.
Changes:
- Treat
splice_command_length == 0xFFFas a legacy sentinel: skip strict command-length enforcement for known commands and reject unknown commands when the length is legacy. - Remove derived wire-length fields from the public
SpliceInfoSectionmodel, and update builder/CLI/tests/docs accordingly. - Tighten parsing validation by erroring when parsed command/descriptor data exceeds the declared lengths (when lengths are not legacy).
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.rs | Removes derived length fields from SpliceInfoSection; adds SpliceCommand::command_type() helper. |
| src/parser.rs | Implements legacy splice_command_length handling and stricter length validation for commands/descriptors. |
| src/commands.rs | Adds legacy-length constant and rejects unknown commands when length is the legacy sentinel. |
| src/tests.rs | Adds regression tests for legacy command length behavior and updates assertions to the new API. |
| src/main.rs | Updates CLI text output to reflect removal of derived length fields; improves unknown command display. |
| src/encoding/round_trip_tests.rs | Updates encoding tests for the cleaned SpliceInfoSection API (no public length fields). |
| src/encoding/commands.rs | Minor cleanup of SpliceCommand::Unknown encoding branch. |
| src/builders/splice_info_section.rs | Updates builder to stop populating removed length fields and derive splice_command_type from command. |
| src/builders/extensions.rs | Routes command-type conversion through the new command_type() method. |
| src/builders/tests.rs | Updates builder tests to stop asserting removed length fields. |
| README.md | Updates docs/examples/JSON output to match the cleaned API and legacy length behavior. |
| examples/builder_demo.rs | Updates example output to avoid removed length fields. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rafaelcaricio
marked this pull request as ready for review
June 26, 2026 12:30
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
splice_command_length == 0xFFFby ignoring the length for known splice commandsSpliceInfoSectionmodel while keepingsplice_command_typeWhy
SCTE-35/2023r1 defines
0xFFFas a backwards-compatible sentinel forsplice_command_lengththat downstream equipment should ignore. The parser previously treated it as a 4095-byte command length, which could push parsing past the actual command and fail before reading descriptors.Validation
cargo fmtcargo test --all-featuresFixes #9