New parser #10
Merged
New parser #10
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
oftheforest
approved these changes
Aug 24, 2023
This version of the Mp4Box trait only has a size() item. In particular, it is missing a getter for the box's type. Furthermore, size() does not include the size of the type field. All of this should be fixed. The proc-macro crate is of primary interest: the main library crate so far contains only the Mp4Box trait declaration, and nonsense/fake box type declarations used to test that the proc macro works. The latter should be replaced with some (simple) actual box types from the standard once the proc macro is functional enough to handle them.
Add a type_() method to the trait and derive macro. Fix size() to include the size of the type field. Extra changes: * Write docs for the Mp4Box trait * Mark the generated trait impls as #[automatically_derived] * Emit a compiler error instead of panicking in the derive macro * Make the test names better
Treating box sizes as a plain u64 is error-prone (speaking from experience); having a separate type like this helps ensure that the special cases (`size: 0`, `size: 1`) are handled correctly and helps prevent invalid states from being constructed.
…plus some convenience functions; and derive them in the derive macro. Drop derive support for enums for now: there is currently no way to use attributes to indicate how enum discriminants are serialized. This will be added later, and enum support can be restored then.
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 merges @oftheforest's new proc-macro-based parser to replace the manually written (de)serialization code in
ParseBox/ParsedBoximpls with derived impls. This leaves room to be able to parse more types of boxes in a less error-prone way for upcoming (de)muxing (#9) and sanitization features.To use the new parser in
mp4san, theMp4Boxtrait in @oftheforest's new parser implementation was replaced withmp4san's existingParseBox/ParsedBox, which means usingBuf/BytesMutfor parsing instead ofstd::iofor now. Theisom_parsemodule was merged intomp4san::parsefor tighter integration for now, but the entireparsemodule can be split off to a new crate in the future when it's more stable.Closes #4.