Skip to content
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 11 commits into from
Apr 21, 2023

Conversation

FreezyLemon
Copy link
Contributor

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.

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
Copy link
Member

@Luni-4 Luni-4 left a 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

src/demuxer.rs Outdated Show resolved Hide resolved
src/ebml/macros.rs Outdated Show resolved Hide resolved
Copy link
Member

@Luni-4 Luni-4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot!

@Luni-4 Luni-4 merged commit 185f93f into rust-av:master Apr 21, 2023
@FreezyLemon FreezyLemon deleted the fix-permutation-new branch April 21, 2023 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants