-
Notifications
You must be signed in to change notification settings - Fork 47
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
Add a callback to select which element are written #173
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -475,18 +475,18 @@ EbmlElement *EbmlElement::CreateElementUsingContext(const EbmlId & aID, const Eb | |
/*! | ||
\todo verify that the size written is the same as the data written | ||
*/ | ||
filepos_t EbmlElement::Render(IOCallback & output, bool bWithDefault, bool bKeepPosition, bool bForceRender) | ||
filepos_t EbmlElement::Render(IOCallback & output, ShouldWrite writeFilter, bool bKeepPosition, bool bForceRender) | ||
{ | ||
assert(bValueIsSet || (bWithDefault && DefaultISset())); // an element is been rendered without a value set !!! | ||
assert(bValueIsSet || writeFilter(*this)); // an element is been rendered without a value set !!! | ||
// it may be a mandatory element without a default value | ||
if (!bWithDefault && IsDefaultValue()) { | ||
if (!writeFilter(*this)) { | ||
return 0; | ||
} | ||
#if !defined(NDEBUG) | ||
std::uint64_t SupposedSize = UpdateSize(bWithDefault, bForceRender); | ||
std::uint64_t SupposedSize = UpdateSize(writeFilter, bForceRender); | ||
#endif // !NDEBUG | ||
filepos_t result = RenderHead(output, bForceRender, bWithDefault, bKeepPosition); | ||
const std::uint64_t WrittenSize = RenderData(output, bForceRender, bWithDefault); | ||
filepos_t result = RenderHead(output, bForceRender, writeFilter, bKeepPosition); | ||
const std::uint64_t WrittenSize = RenderData(output, bForceRender, writeFilter); | ||
#if !defined(NDEBUG) | ||
if (static_cast<std::int64_t>(SupposedSize) != (0-1)) | ||
assert(WrittenSize == SupposedSize); | ||
|
@@ -500,12 +500,12 @@ filepos_t EbmlElement::Render(IOCallback & output, bool bWithDefault, bool bKeep | |
\todo handle exceptions on errors | ||
\todo handle CodeSize bigger than 5 bytes | ||
*/ | ||
filepos_t EbmlElement::RenderHead(IOCallback & output, bool bForceRender, bool bWithDefault, bool bKeepPosition) | ||
filepos_t EbmlElement::RenderHead(IOCallback & output, bool bForceRender, ShouldWrite writeFilter, bool bKeepPosition) | ||
{ | ||
if (EBML_ID_LENGTH((const EbmlId&)*this) <= 0 || EBML_ID_LENGTH((const EbmlId&)*this) > 4) | ||
return 0; | ||
|
||
UpdateSize(bWithDefault, bForceRender); | ||
UpdateSize(writeFilter, bForceRender); | ||
|
||
return MakeRenderHead(output, bKeepPosition); | ||
} | ||
|
@@ -531,9 +531,9 @@ filepos_t EbmlElement::MakeRenderHead(IOCallback & output, bool bKeepPosition) | |
return FinalHeadSize; | ||
} | ||
|
||
std::uint64_t EbmlElement::ElementSize(bool bWithDefault) const | ||
std::uint64_t EbmlElement::ElementSize(ShouldWrite writeFilter) const | ||
{ | ||
if (!bWithDefault && IsDefaultValue()) | ||
if (!writeFilter(*this)) | ||
return 0; // won't be saved | ||
return Size + EBML_ID_LENGTH((const EbmlId&)*this) + CodedSizeLength(Size, SizeLength, bSizeIsFinite); | ||
} | ||
|
@@ -602,21 +602,21 @@ filepos_t EbmlElement::OverwriteData(IOCallback & output, bool bKeepPosition) | |
|
||
auto CurrentPosition = output.getFilePointer(); | ||
output.setFilePointer(GetElementPosition() + HeaderSize); | ||
auto Result = RenderData(output, true, bKeepPosition); | ||
auto Result = RenderData(output, true, bKeepPosition ? WriteAll : WriteSkipDefault); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the intent behind this change. What does keeping the position have to do with the question whether or not to write the element? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is just the translation of the previous call into the new system. The original logic may be flawed and it should be fixed separately. In fact when overwriting you'd probably want to tell which elements you want to write (the least possible). This whole API is a recipe for broken files (which originally was a welcome possibility in libebml to be able to generate broken files to test against). It seems mkvtoolnix doesn't use it, there's only a test calling OverwriteHead(). So I'd rather remove the API altogether. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, I see. Thanks for the explanation. Yeah, MKVToolNix (or rather: It never uses |
||
output.setFilePointer(CurrentPosition); | ||
assert(Result == DataSize); | ||
return Result; | ||
} | ||
|
||
|
||
std::uint64_t EbmlElement::VoidMe(IOCallback & output, bool bWithDefault) const | ||
std::uint64_t EbmlElement::VoidMe(IOCallback & output, ShouldWrite writeFilter) const | ||
{ | ||
if (ElementPosition == 0) { | ||
return 0; // the element has not been written | ||
} | ||
|
||
EbmlVoid Dummy; | ||
return Dummy.Overwrite(*this, output, true, bWithDefault); | ||
return Dummy.Overwrite(*this, output, true, writeFilter); | ||
} | ||
|
||
} // namespace libebml |
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.
I… don't like this
#define
. Consumers can already use the version number#defines
in order to differentiate between v1 & v2 of the library.Not sure how I'll handle it in MKVToolNix. Will have to look into how many places will have to be changed as I'm generally not a fan of a lot of conditional compilation anyway. Maybe I'll just throw the switch & require v2 as soon as I add support for it.
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.
The define is there to tell v2 users which API they can expect. Once v2 is stable we can remove it and everyone should assume it's there for v2 code. In other words it's to make your life easier if you want to support the old and new API in your codebase for a while. I don't mind removing it and you handle it on your side.
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.
Makes sense, but isn't needed, at least not for me. As there's no official release of v2 yet, I won't support different snapshots of the upcoming v2 API in MKVToolNix. I'll try my best to support v1 & the latest v2 simultaneously, but even that is in question with this RFC.