From efa67d6a1a6d68939e29384534b2d34b5ea8776d Mon Sep 17 00:00:00 2001 From: Martchus Date: Wed, 7 Mar 2018 00:30:08 +0100 Subject: [PATCH] Improve siblingById() and subelementByPath() * Use 2 functions instead of flag parameter * Support const correctness --- genericfileelement.h | 116 +++++++++++++++++++++++++++++++-- matroska/matroskacontainer.cpp | 2 +- mp4/mp4container.cpp | 22 +++---- mp4/mp4track.cpp | 16 ++--- 4 files changed, 129 insertions(+), 27 deletions(-) diff --git a/genericfileelement.h b/genericfileelement.h index bdc2981..0d46e1d 100644 --- a/genericfileelement.h +++ b/genericfileelement.h @@ -103,8 +103,14 @@ public: const ImplementationType* lastChild() const; ImplementationType* subelementByPath(Diagnostics &diag, IdentifierType item); ImplementationType* subelementByPath(Diagnostics &diag, IdentifierType item, IdentifierType remainingPath...); + const ImplementationType* subelementByPath(Diagnostics &diag, IdentifierType item) const; + const ImplementationType* subelementByPath(Diagnostics &diag, IdentifierType item, IdentifierType remainingPath...) const; ImplementationType* childById(const IdentifierType &id, Diagnostics &diag); - ImplementationType* siblingById(const IdentifierType &id, Diagnostics &diag, bool includeThis = false); + const ImplementationType* childById(const IdentifierType &id, Diagnostics &diag) const; + ImplementationType* siblingById(const IdentifierType &id, Diagnostics &diag); + const ImplementationType* siblingById(const IdentifierType &id, Diagnostics &diag) const; + ImplementationType* siblingByIdIncludingThis(const IdentifierType &id, Diagnostics &diag); + const ImplementationType* siblingByIdIncludingThis(const IdentifierType &id, Diagnostics &diag) const; bool isParent() const; bool isPadding() const; uint64 firstChildOffset() const; @@ -574,6 +580,36 @@ ImplementationType *GenericFileElement::subelementByPath(Dia return nullptr; } +/*! + * \brief Returns the sub element for the specified path. + * + * The current element keeps ownership over the returned element. + * If no element could be found nullptr is returned. + * + * \throws Throws a parsing exception when a parsing error occurs. + * \throws Throws std::ios_base::failure when an IO error occurs. + */ +template +const ImplementationType *GenericFileElement::subelementByPath(Diagnostics &diag, IdentifierType item) const +{ + return const_cast *>(this)->subelementByPath(diag, item); +} + +/*! + * \brief Returns the sub element for the specified path. + * + * The current element keeps ownership over the returned element. + * If no element could be found nullptr is returned. + * + * \throws Throws a parsing exception when a parsing error occurs. + * \throws Throws std::ios_base::failure when an IO error occurs. + */ +template +const ImplementationType *GenericFileElement::subelementByPath(Diagnostics &diag, IdentifierType item, IdentifierType remainingPath...) const +{ + return const_cast *>(this)->subelementByPath(diag, item, remainingPath); +} + /*! * \brief Returns the first child with the specified \a id. * @@ -597,11 +633,22 @@ ImplementationType *GenericFileElement::childById(const Iden } /*! - * \brief Returns the first sibling with the specified \a id. + * \brief Returns the first child with the specified \a id. * - * \param id Specifies the id of the sibling to be returned. - * \param includeThis Indicates whether this instance should be returned - * if it has the specified \a id. + * The current element keeps ownership over the returned element. + * If no element could be found nullptr is returned. + * + * \throws Throws a parsing exception when a parsing error occurs. + * \throws Throws std::ios_base::failure when an IO error occurs. + */ +template +const ImplementationType *GenericFileElement::childById(const IdentifierType &id, Diagnostics &diag) const +{ + return const_cast *>(this)->childById(id, diag); +} + +/*! + * \brief Returns the first sibling with the specified \a id. * * The current element keeps ownership over the returned element. * If no element could be found nullptr is returned. @@ -611,10 +658,10 @@ ImplementationType *GenericFileElement::childById(const Iden * \throws Throws std::ios_base::failure when an IO error occurs. */ template -ImplementationType *GenericFileElement::siblingById(const IdentifierType &id, Diagnostics &diag, bool includeThis) +ImplementationType *GenericFileElement::siblingById(const IdentifierType &id, Diagnostics &diag) { parse(diag); // ensure element is parsed - for(ImplementationType *sibling = includeThis ? static_cast(this) : nextSibling(); sibling; sibling = sibling->nextSibling()) { + for(ImplementationType *sibling = nextSibling(); sibling; sibling = sibling->nextSibling()) { sibling->parse(diag); if(sibling->id() == id) { return sibling; @@ -623,6 +670,61 @@ ImplementationType *GenericFileElement::siblingById(const Id return nullptr; } +/*! + * \brief Returns the first sibling with the specified \a id. + * + * The current element keeps ownership over the returned element. + * If no element could be found nullptr is returned. + * Possibly returns a pointer to the current instance (see \a includeThis). + * + * \throws Throws a parsing exception when a parsing error occurs. + * \throws Throws std::ios_base::failure when an IO error occurs. + */ +template +const ImplementationType *GenericFileElement::siblingById(const IdentifierType &id, Diagnostics &diag) const +{ + return const_cast *>(this)->siblingById(id, diag); +} + +/*! + * \brief Returns the first sibling with the specified \a id or the current instance if its ID equals \a id. + * + * The current element keeps ownership over the returned element. + * If no element could be found nullptr is returned. + * Possibly returns a pointer to the current instance (see \a includeThis). + * + * \throws Throws a parsing exception when a parsing error occurs. + * \throws Throws std::ios_base::failure when an IO error occurs. + */ +template +ImplementationType *GenericFileElement::siblingByIdIncludingThis(const IdentifierType &id, Diagnostics &diag) +{ + parse(diag); // ensure element is parsed + for(ImplementationType *sibling = static_cast(this); sibling; sibling = sibling->nextSibling()) { + sibling->parse(diag); + if(sibling->id() == id) { + return sibling; + } + } + return nullptr; +} + +/*! + * \brief Returns the first sibling with the specified \a id or the current instance if its ID equals \a id. + * + * The current element keeps ownership over the returned element. + * If no element could be found nullptr is returned. + * Possibly returns a pointer to the current instance (see \a includeThis). + * + * \throws Throws a parsing exception when a parsing error occurs. + * \throws Throws std::ios_base::failure when an IO error occurs. + */ +template +const ImplementationType *GenericFileElement::siblingByIdIncludingThis(const IdentifierType &id, Diagnostics &diag) const +{ + return const_cast *>(this)->siblingByIdIncludingThis(id, diag); +} + /*! * \brief Returns an indication whether this instance is a parent element. */ diff --git a/matroska/matroskacontainer.cpp b/matroska/matroskacontainer.cpp index bcb330d..2ca79db 100644 --- a/matroska/matroskacontainer.cpp +++ b/matroska/matroskacontainer.cpp @@ -342,7 +342,7 @@ ElementPosition MatroskaContainer::determineElementPosition(uint64 elementId, Di if(!m_firstElement || m_segmentCount != 1) { return ElementPosition::Keep; } - const auto *const segmentElement = m_firstElement->siblingById(MatroskaIds::Segment, diag, true); + const auto *const segmentElement = m_firstElement->siblingByIdIncludingThis(MatroskaIds::Segment, diag); if(!segmentElement) { return ElementPosition::Keep; } diff --git a/mp4/mp4container.cpp b/mp4/mp4container.cpp index 3baf9c6..1b25ae8 100644 --- a/mp4/mp4container.cpp +++ b/mp4/mp4container.cpp @@ -75,7 +75,7 @@ void Mp4Container::internalParseHeader(Diagnostics &diag) //const string context("parsing header of MP4 container"); will be used when generating notifications m_firstElement = make_unique(*this, startOffset()); m_firstElement->parse(diag); - Mp4Atom *ftypAtom = m_firstElement->siblingById(Mp4AtomIds::FileType, diag, true); + Mp4Atom *ftypAtom = m_firstElement->siblingByIdIncludingThis(Mp4AtomIds::FileType, diag); if(ftypAtom) { stream().seekg(static_cast(ftypAtom->dataOffset())); m_doctype = reader().readString(4); @@ -100,7 +100,7 @@ void Mp4Container::internalParseTags(Diagnostics &diag) } catch(const NoDataFoundException &) { m_tags.pop_back(); } - metaAtom = metaAtom->siblingById(Mp4AtomIds::Meta, diag, false); + metaAtom = metaAtom->siblingById(Mp4AtomIds::Meta, diag); if(metaAtom) { surplusMetaAtoms = true; } @@ -119,7 +119,7 @@ void Mp4Container::internalParseTracks(Diagnostics &diag) static const string context("parsing tracks of MP4 container"); try { // get moov atom which holds track information - if(Mp4Atom *moovAtom = firstElement()->siblingById(Mp4AtomIds::Movie, diag, true)) { + if(Mp4Atom *moovAtom = firstElement()->siblingByIdIncludingThis(Mp4AtomIds::Movie, diag)) { // get mvhd atom which holds overall track information if(Mp4Atom *mvhdAtom = moovAtom->childById(Mp4AtomIds::MovieHeader, diag)) { if(mvhdAtom->dataSize() > 0) { @@ -191,7 +191,7 @@ void Mp4Container::internalParseTracks(Diagnostics &diag) } catch(const Failure &) { diag.emplace_back(DiagLevel::Critical, argsToString("Unable to parse track ", trackNum, '.'), context); } - trakAtom = trakAtom->siblingById(Mp4AtomIds::Track, diag, false); // get next trak atom + trakAtom = trakAtom->siblingById(Mp4AtomIds::Track, diag); // get next trak atom ++trackNum; } // get overall duration, creation time and modification time if not determined yet @@ -263,7 +263,7 @@ void Mp4Container::internalMakeFile(Diagnostics &diag, AbortableProgressFeedback Mp4Atom *level0Atom, *level1Atom, *level2Atom, *lastAtomToBeWritten; try { // file type atom (mandatory) - if((fileTypeAtom = firstElement()->siblingById(Mp4AtomIds::FileType, diag, true))) { + if((fileTypeAtom = firstElement()->siblingByIdIncludingThis(Mp4AtomIds::FileType, diag))) { // buffer atom fileTypeAtom->makeBuffer(); } else { @@ -273,13 +273,13 @@ void Mp4Container::internalMakeFile(Diagnostics &diag, AbortableProgressFeedback } // progressive download information atom (not mandatory) - if((progressiveDownloadInfoAtom = firstElement()->siblingById(Mp4AtomIds::ProgressiveDownloadInformation, diag, true))) { + if((progressiveDownloadInfoAtom = firstElement()->siblingByIdIncludingThis(Mp4AtomIds::ProgressiveDownloadInformation, diag))) { // buffer atom progressiveDownloadInfoAtom->makeBuffer(); } // movie atom (mandatory) - if(!(movieAtom = firstElement()->siblingById(Mp4AtomIds::Movie, diag, true))) { + if(!(movieAtom = firstElement()->siblingByIdIncludingThis(Mp4AtomIds::Movie, diag))) { // throw error if missing diag.emplace_back(DiagLevel::Critical, "Mandatory \"moov\"-atom not in the source file found.", context); throw InvalidDataException(); @@ -868,16 +868,16 @@ void Mp4Container::updateOffsets(const std::vector &oldMdatOffsets, const } // update "base-data-offset-present" of "tfhd"-atom (NOT tested properly) try { - for(Mp4Atom *moofAtom = firstElement()->siblingById(Mp4AtomIds::MovieFragment, diag, false); - moofAtom; moofAtom = moofAtom->siblingById(Mp4AtomIds::MovieFragment, diag, false)) { + for(Mp4Atom *moofAtom = firstElement()->siblingById(Mp4AtomIds::MovieFragment, diag); + moofAtom; moofAtom = moofAtom->siblingById(Mp4AtomIds::MovieFragment, diag)) { moofAtom->parse(diag); try { for(Mp4Atom *trafAtom = moofAtom->childById(Mp4AtomIds::TrackFragment, diag); trafAtom; - trafAtom = trafAtom->siblingById(Mp4AtomIds::TrackFragment, diag, false)) { + trafAtom = trafAtom->siblingById(Mp4AtomIds::TrackFragment, diag)) { trafAtom->parse(diag); int tfhdAtomCount = 0; for(Mp4Atom *tfhdAtom = trafAtom->childById(Mp4AtomIds::TrackFragmentHeader, diag); tfhdAtom; - tfhdAtom = tfhdAtom->siblingById(Mp4AtomIds::TrackFragmentHeader, diag, false)) { + tfhdAtom = tfhdAtom->siblingById(Mp4AtomIds::TrackFragmentHeader, diag)) { tfhdAtom->parse(diag); ++tfhdAtomCount; if(tfhdAtom->dataSize() >= 8) { diff --git a/mp4/mp4track.cpp b/mp4/mp4track.cpp index ce60e1d..35da101 100644 --- a/mp4/mp4track.cpp +++ b/mp4/mp4track.cpp @@ -214,11 +214,11 @@ std::vector Mp4Track::readChunkOffsets(bool parseFragments, Diagnostics // read sample offsets of fragments if(parseFragments) { uint64 totalDuration = 0; - for(Mp4Atom *moofAtom = m_trakAtom->container().firstElement()->siblingById(Mp4AtomIds::MovieFragment, diag, true); moofAtom; moofAtom = moofAtom->siblingById(Mp4AtomIds::MovieFragment, diag, false)) { + for(Mp4Atom *moofAtom = m_trakAtom->container().firstElement()->siblingByIdIncludingThis(Mp4AtomIds::MovieFragment, diag); moofAtom; moofAtom = moofAtom->siblingById(Mp4AtomIds::MovieFragment, diag)) { moofAtom->parse(diag); - for(Mp4Atom *trafAtom = moofAtom->childById(Mp4AtomIds::TrackFragment, diag); trafAtom; trafAtom = trafAtom->siblingById(Mp4AtomIds::TrackFragment, diag, false)) { + for(Mp4Atom *trafAtom = moofAtom->childById(Mp4AtomIds::TrackFragment, diag); trafAtom; trafAtom = trafAtom->siblingById(Mp4AtomIds::TrackFragment, diag)) { trafAtom->parse(diag); - for(Mp4Atom *tfhdAtom = trafAtom->childById(Mp4AtomIds::TrackFragmentHeader, diag); tfhdAtom; tfhdAtom = tfhdAtom->siblingById(Mp4AtomIds::TrackFragmentHeader, diag, false)) { + for(Mp4Atom *tfhdAtom = trafAtom->childById(Mp4AtomIds::TrackFragmentHeader, diag); tfhdAtom; tfhdAtom = tfhdAtom->siblingById(Mp4AtomIds::TrackFragmentHeader, diag)) { tfhdAtom->parse(diag); uint32 calculatedDataSize = 0; if(tfhdAtom->dataSize() < calculatedDataSize) { @@ -271,7 +271,7 @@ std::vector Mp4Track::readChunkOffsets(bool parseFragments, Diagnostics inputStream().seekg(4, ios_base::cur); } } - for(Mp4Atom *trunAtom = trafAtom->childById(Mp4AtomIds::TrackFragmentRun, diag); trunAtom; trunAtom = trunAtom->siblingById(Mp4AtomIds::TrackFragmentRun, diag, false)) { + for(Mp4Atom *trunAtom = trafAtom->childById(Mp4AtomIds::TrackFragmentRun, diag); trunAtom; trunAtom = trunAtom->siblingById(Mp4AtomIds::TrackFragmentRun, diag)) { uint32 calculatedDataSize = 8; if(trunAtom->dataSize() < calculatedDataSize) { diag.emplace_back(DiagLevel::Critical, "trun atom is truncated.", context); @@ -1786,11 +1786,11 @@ void Mp4Track::internalParseHeader(Diagnostics &diag) // no sample sizes found, search for trun atoms uint64 totalDuration = 0; - for(Mp4Atom *moofAtom = m_trakAtom->container().firstElement()->siblingById(MovieFragment, diag, true); moofAtom; moofAtom = moofAtom->siblingById(MovieFragment, diag, false)) { + for(Mp4Atom *moofAtom = m_trakAtom->container().firstElement()->siblingByIdIncludingThis(MovieFragment, diag); moofAtom; moofAtom = moofAtom->siblingById(MovieFragment, diag)) { moofAtom->parse(diag); - for(Mp4Atom *trafAtom = moofAtom->childById(TrackFragment, diag); trafAtom; trafAtom = trafAtom->siblingById(TrackFragment, diag, false)) { + for(Mp4Atom *trafAtom = moofAtom->childById(TrackFragment, diag); trafAtom; trafAtom = trafAtom->siblingById(TrackFragment, diag)) { trafAtom->parse(diag); - for(Mp4Atom *tfhdAtom = trafAtom->childById(TrackFragmentHeader, diag); tfhdAtom; tfhdAtom = tfhdAtom->siblingById(TrackFragmentHeader, diag, false)) { + for(Mp4Atom *tfhdAtom = trafAtom->childById(TrackFragmentHeader, diag); tfhdAtom; tfhdAtom = tfhdAtom->siblingById(TrackFragmentHeader, diag)) { tfhdAtom->parse(diag); uint32 calculatedDataSize = 0; if(tfhdAtom->dataSize() < calculatedDataSize) { @@ -1842,7 +1842,7 @@ void Mp4Track::internalParseHeader(Diagnostics &diag) m_istream->seekg(4, ios_base::cur); } } - for(Mp4Atom *trunAtom = trafAtom->childById(TrackFragmentRun, diag); trunAtom; trunAtom = trunAtom->siblingById(TrackFragmentRun, diag, false)) { + for(Mp4Atom *trunAtom = trafAtom->childById(TrackFragmentRun, diag); trunAtom; trunAtom = trunAtom->siblingById(TrackFragmentRun, diag)) { uint32 calculatedDataSize = 8; if(trunAtom->dataSize() < calculatedDataSize) { diag.emplace_back(DiagLevel::Critical, "trun atom is truncated.", context);