Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements support for multiple EXT-X-KEY tags per segment in accordance with the HLS specification, which allows segments to have multiple encryption keys with different KEYFORMAT attributes for Common Encryption support.
Changes:
- Modified the parser to track multiple keys per segment instead of a single key
- Updated the Segment model to store a list of keys while maintaining backward compatibility through a
keyproperty - Added comprehensive test coverage for parsing and round-trip serialization of multi-key playlists
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_parser.py | Updated tests to access segment keys via keys list and added new test for multiple KEYFORMAT support |
| tests/test_model.py | Added tests for segment.keys list access, backward compatibility of segment.key property, and round-trip preservation |
| tests/playlists.py | Added test playlist with multiple EXT-X-KEY tags using different KEYFORMAT attributes |
| openm3u8/parser.py | Changed parser state from single current_key to current_keys list with KEYFORMAT-aware replacement logic |
| openm3u8/model.py | Updated Segment class to use keys list internally with key property for backward compatibility, modified dumps() and by_key() methods |
| openm3u8/_m3u8_parser.c | Implemented C parser changes mirroring Python parser: current_keys list, KEYFORMAT comparison, and key replacement logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return [segment for segment in self if segment.key == key] | ||
| if key is None: | ||
| return [segment for segment in self if not segment.keys] | ||
| return [segment for segment in self if key in segment.keys] |
There was a problem hiding this comment.
The by_key method performs a linear search through segments and checks key membership using in operator on a list, which is O(n*m) where n is segment count and m is keys per segment. For better performance, consider converting segment.keys to a set before the membership check, or if keys are unique objects, compare by key attributes rather than object identity.
| return [segment for segment in self if key in segment.keys] | |
| return [segment for segment in self if key in set(segment.keys)] |
There was a problem hiding this comment.
Robot, it's strictly worse to create a new set every time.
The HLS spec says:
This PR updates the package such that multiple keys are supported.
If others can test this, I'll release it as a new major version!