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

[RFC] Store the default value in a union inside EbmlSemantic #156

Closed
wants to merge 5 commits into from

Conversation

robUx4
Copy link
Contributor

@robUx4 robUx4 commented Dec 17, 2023

A template might be cleaner, more type-safe. I gave it a try but it seemed very instrusive (all classes need to be touched).

This provides a basic system to set the default value in the semantic. If approve we can change how we set default values in libmatroska, using the new macros. And then we can switch the access to the default value to this storage in EbmlSemantic.

@robUx4 robUx4 added api-break breaks the API (e.g. programs using it will have to adjust their source code) abi-break breaks the ABI (e.g. programs linked against the library have to be recompiled) labels Dec 17, 2023

inline const DefaultValues & DefaultValue() const { return defaultValue; }

#if defined(EBML_STRICT_API)
Copy link
Contributor

Choose a reason for hiding this comment

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

EBML_STRICT_API has been removed with one of the commits over the last couple of days. Just always use private:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, good catch.

Copy link
Contributor

@mbunkus mbunkus left a comment

Choose a reason for hiding this comment

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

As stated in that comment…

@robUx4 robUx4 force-pushed the semantic_default_union branch 3 times, most recently from 79ea49c to c5473a1 Compare December 18, 2023 12:56
@robUx4
Copy link
Contributor Author

robUx4 commented Dec 18, 2023

This MR is just wrong, it should be stored alongside the EBML ID, not the semantic in the parent. And it turns out we already have macros to set the default value.

@robUx4 robUx4 closed this Dec 18, 2023
@robUx4 robUx4 deleted the semantic_default_union branch December 18, 2023 13:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abi-break breaks the ABI (e.g. programs linked against the library have to be recompiled) api-break breaks the API (e.g. programs using it will have to adjust their source code)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants