-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Create declarative macro for EBML Elements #125
Merged
Merged
Conversation
This file contains 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
This makes the Element ID a fixed part of the Error. The ID can probably be turned into an `Option<NonZeroU32>` at some point, but that feels like a low priority.
- Refactor ebml_generic for better readability - Add lifetime parameter to EbmlParsable This allows implementing the trait for borrowed types like &[u8] (binary_ref). - Add `fn has_crc() -> bool` to trait EbmlParsable Also adds CRC-32 capabilities to ebml_generic, making it functionally equivalent to the master helper function when has_crc returns true. - Rename ebml_generic to ebml_element It can now handle most EBML Elements without issue, so the new name is more descriptive.
**This breaks compilation for elements.rs**, because matroska_permutation was changed (again) and it's not worth fixing this when the macros can soon be used instead. The macro `impl_ebml_master` will 1. create a pub struct definition with the given field names/types and any meta (like #[derive] blocks) and 2. implement EbmlParsable for the aforementioned struct. This implementation will set the has_crc() to true (to use the new ebml_element feature) and create a try_parse definition based on matroska_permutation. It will auto-"unwrap" the parser results and handle any parsing errors. It also handles Option<T> and Vec<T>.
There's no such thing as "single-element tuples", so the macro-implemented `impl Permutation` blocks don't work for singular Parser functions. So, here's a manual implementation.
- Use impl_ebml_master to define all Matroska Master Elements - Change sub_element helper to make it similar to ebml_element, but still work in the SegmentElement context. - Adapt segment_element fn to the sub_element changes. - Add note to fix what used to be correct Float Element handling (used to be float_or) - Small fix in the macro (lifetime-related) - Comment out/remove the unimplemented Elements Chapters, Tags, Attachments and Cues (should be added back later) - Edit serializer/muxer/demuxer stuff so it works again after the changes
Luni-4
requested changes
Apr 21, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your great work!
Just two questions and we can land it
They were pub before (and in the crate root).
Luni-4
approved these changes
Apr 21, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot!
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.
This fixes a bug introduced in #123 (see the first commit on this branch).
Float handling (see #116) has regressed since float_or can't currently be used. This should be fixable, but I didn't want to push even more things into the branch.
Something should also be done about the
TrackEntry
field stream_index. It shouldn't really be in that struct since it's not part of the Matroska format. That has an easy workaround though, so it's not very important.Should also get macro-specific tests at some point.