From 8663dedf8c96af7bade6dcb492eb213b43ac454a Mon Sep 17 00:00:00 2001 From: Martchus Date: Sat, 30 Jul 2016 22:35:46 +0200 Subject: [PATCH] Fix misc issues --- CMakeLists.txt | 1 - matroska/matroskatag.cpp | 10 +++------- tagtarget.h | 2 +- tagvalue.cpp | 27 ++++++++++++++++++--------- tagvalue.h | 31 +++++++++++++++++++++++++++++++ tests/overall.cpp | 30 ++++++++++++++++++++++-------- 6 files changed, 75 insertions(+), 26 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0a34b12..9cbb854 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -142,7 +142,6 @@ set(SRC_FILES mediaformat.cpp ) set(TEST_HEADER_FILES - ) set(TEST_SRC_FILES tests/cppunit.cpp diff --git a/matroska/matroskatag.cpp b/matroska/matroskatag.cpp index fde31be..ed32410 100644 --- a/matroska/matroskatag.cpp +++ b/matroska/matroskatag.cpp @@ -89,16 +89,15 @@ void MatroskaTag::parse(EbmlElement &tagElement) static const string context("parsing Matroska tag"); tagElement.parse(); m_size = tagElement.totalSize(); - EbmlElement *child = tagElement.firstChild(); MatroskaTagField field; - while(child) { + for(EbmlElement *child = tagElement.firstChild(); child; child = child->nextSibling()) { child->parse(); switch(child->id()) { case MatroskaIds::SimpleTag: { try { field.invalidateNotifications(); field.reparse(*child, true); - fields().insert(pair(field.id(), field)); + fields().insert(make_pair(field.id(), field)); } catch(const Failure &) { } addNotifications(context, field); @@ -107,7 +106,6 @@ void MatroskaTag::parse(EbmlElement &tagElement) parseTargets(*child); break; } - child = child->nextSibling(); } } @@ -152,8 +150,7 @@ void MatroskaTag::parseTargets(EbmlElement &targetsElement) bool targetTypeValueFound = false; bool targetTypeFound = false; targetsElement.parse(); - EbmlElement *child = targetsElement.firstChild(); - while(child) { + for(EbmlElement *child = targetsElement.firstChild(); child; child = child->nextSibling()) { try { child->parse(); } catch(const Failure &) { @@ -192,7 +189,6 @@ void MatroskaTag::parseTargets(EbmlElement &targetsElement) default: addNotification(NotificationType::Warning, "Targets element contains unknown element. It will be ignored.", context); } - child = child->nextSibling(); } if(!m_target.level()) { m_target.setLevel(50); // default level diff --git a/tagtarget.h b/tagtarget.h index eff622e..58bd712 100644 --- a/tagtarget.h +++ b/tagtarget.h @@ -198,11 +198,11 @@ inline void TagTarget::clear() /*! * \brief Returns whether the tag targets are equal. + * \remarks Targets where only the level name differs are considered equal though. */ inline bool TagTarget::operator ==(const TagTarget &other) const { return level() == other.level() - //&& m_levelName == other.m_levelName // consider targets with the same level number but different level names equal? && m_tracks == other.m_tracks && m_chapters == other.m_chapters && m_editions == other.m_editions diff --git a/tagvalue.cpp b/tagvalue.cpp index 737159e..5d7efd2 100644 --- a/tagvalue.cpp +++ b/tagvalue.cpp @@ -226,7 +226,15 @@ PositionInSet TagValue::toPositionInSet() const if(!isEmpty()) { switch(m_type) { case TagDataType::Text: - return PositionInSet(string(m_ptr.get(), m_size)); + switch(m_encoding) { + case TagTextEncoding::Unspecified: + case TagTextEncoding::Latin1: + case TagTextEncoding::Utf8: + return PositionInSet(string(m_ptr.get(), m_size)); + case TagTextEncoding::Utf16LittleEndian: + case TagTextEncoding::Utf16BigEndian: + return PositionInSet(u16string(reinterpret_cast(m_ptr.get()), m_size / 2)); + } case TagDataType::Integer: case TagDataType::PositionInSet: switch(m_size) { @@ -468,19 +476,20 @@ void TagValue::toWString(std::u16string &result, TagTextEncoding encoding) const /*! * \brief Assigns a copy of the given \a text. * \param text Specifies the text to be assigned. + * \param textSize Specifies the size of \a text. (The actual number of bytes, not the number of characters.) * \param textEncoding Specifies the encoding of the given \a text. * \param convertTo Specifies the encoding to convert \a text to; set to TagTextEncoding::Unspecified to * use \a textEncoding without any character set conversions. * \throws Throws a ConversionException if the conversion the specified character set fails. */ -void TagValue::assignText(const string &text, TagTextEncoding textEncoding, TagTextEncoding convertTo) +void TagValue::assignText(const char *text, std::size_t textSize, TagTextEncoding textEncoding, TagTextEncoding convertTo) { m_type = TagDataType::Text; m_encoding = textEncoding; - if(!text.empty()) { + if(textSize) { if(convertTo == TagTextEncoding::Unspecified || textEncoding == convertTo) { - m_ptr = make_unique(m_size = text.size()); - text.copy(m_ptr.get(), m_size); + m_ptr = make_unique(m_size = textSize); + copy(text, text + textSize, m_ptr.get()); } else { StringData encodedData; switch(textEncoding) { @@ -488,13 +497,13 @@ void TagValue::assignText(const string &text, TagTextEncoding textEncoding, TagT // use pre-defined methods when encoding to UTF-8 switch(convertTo) { case TagTextEncoding::Latin1: - encodedData = convertLatin1ToUtf8(text.data(), text.size()); + encodedData = convertLatin1ToUtf8(text, textSize); break; case TagTextEncoding::Utf16LittleEndian: - encodedData = convertUtf16LEToUtf8(text.data(), text.size()); + encodedData = convertUtf16LEToUtf8(text, textSize); break; case TagTextEncoding::Utf16BigEndian: - encodedData = convertUtf16BEToUtf8(text.data(), text.size()); + encodedData = convertUtf16BEToUtf8(text, textSize); break; default: ; @@ -504,7 +513,7 @@ void TagValue::assignText(const string &text, TagTextEncoding textEncoding, TagT // otherwise, determine input and output parameter to use general covertString method const auto inputParameter = encodingParameter(textEncoding); const auto outputParameter = encodingParameter(convertTo); - encodedData = convertString(inputParameter.first, outputParameter.first, text.data(), text.size(), outputParameter.second / inputParameter.second); + encodedData = convertString(inputParameter.first, outputParameter.first, text, textSize, outputParameter.second / inputParameter.second); } } // can't just move the encoded data because it needs to be deleted with free diff --git a/tagvalue.h b/tagvalue.h index 2a4dcc9..de20199 100644 --- a/tagvalue.h +++ b/tagvalue.h @@ -64,6 +64,7 @@ class LIB_EXPORT TagValue public: // constructor, destructor TagValue(); + TagValue(const char *text, std::size_t textSize, TagTextEncoding textEncoding = TagTextEncoding::Latin1, TagTextEncoding convertTo = TagTextEncoding::Unspecified); TagValue(const std::string &text, TagTextEncoding textEncoding = TagTextEncoding::Latin1, TagTextEncoding convertTo = TagTextEncoding::Unspecified); TagValue(int value); TagValue(const char *data, size_t length, TagDataType type = TagDataType::Undefined, TagTextEncoding encoding = TagTextEncoding::Latin1); @@ -107,6 +108,7 @@ public: TagTextEncoding descriptionEncoding() const; static const TagValue &empty(); + void assignText(const char *text, std::size_t textSize, TagTextEncoding textEncoding = TagTextEncoding::Latin1, TagTextEncoding convertTo = TagTextEncoding::Unspecified); void assignText(const std::string &text, TagTextEncoding textEncoding = TagTextEncoding::Latin1, TagTextEncoding convertTo = TagTextEncoding::Unspecified); void assignInteger(int value); void assignStandardGenreIndex(int index); @@ -140,6 +142,22 @@ inline TagValue::TagValue() : m_descEncoding(TagTextEncoding::Latin1) {} +/*! + * \brief Constructs a new TagValue holding a copy of the given \a text. + * \param text Specifies the text to be assigned. + * \param textSize Specifies the size of \a text. (The actual number of bytes, not the number of characters.) + * \param textEncoding Specifies the encoding of the given \a text. + * \param convertTo Specifies the encoding to convert \a text to; set to TagTextEncoding::Unspecified to + * use \a textEncoding without any character set conversions. + * \throws Throws a ConversionException if the conversion the specified character set fails. + */ +inline TagValue::TagValue(const char *text, std::size_t textSize, TagTextEncoding textEncoding, TagTextEncoding convertTo) : + m_labeledAsReadonly(false), + m_descEncoding(TagTextEncoding::Latin1) +{ + assignText(text, textSize, textEncoding, convertTo); +} + /*! * \brief Constructs a new TagValue holding a copy of the given \a text. * \param text Specifies the text to be assigned. @@ -215,6 +233,19 @@ inline TagValue::TagValue(const PositionInSet &value) : TagValue(reinterpret_cast(&value), sizeof(value), TagDataType::PositionInSet) {} +/*! + * \brief Assigns a copy of the given \a text. + * \param text Specifies the text to be assigned. + * \param textEncoding Specifies the encoding of the given \a text. + * \param convertTo Specifies the encoding to convert \a text to; set to TagTextEncoding::Unspecified to + * use \a textEncoding without any character set conversions. + * \throws Throws a ConversionException if the conversion the specified character set fails. + */ +inline void TagValue::assignText(const std::string &text, TagTextEncoding textEncoding, TagTextEncoding convertTo) +{ + assignText(text.data(), text.size(), textEncoding, convertTo); +} + /*! * \brief Assigns the given PositionInSet \a value. */ diff --git a/tests/overall.cpp b/tests/overall.cpp index d9d2b97..9c21b01 100644 --- a/tests/overall.cpp +++ b/tests/overall.cpp @@ -44,15 +44,17 @@ class OverallTests : public TestFixture { CPPUNIT_TEST_SUITE(OverallTests); CPPUNIT_TEST(testMp4Parsing); - CPPUNIT_TEST(testMp4Making); CPPUNIT_TEST(testMp3Parsing); - CPPUNIT_TEST(testMp3Making); CPPUNIT_TEST(testOggParsing); - CPPUNIT_TEST(testOggMaking); CPPUNIT_TEST(testFlacParsing); - CPPUNIT_TEST(testFlacMaking); CPPUNIT_TEST(testMkvParsing); +#ifdef PLATFORM_UNIX + CPPUNIT_TEST(testMp4Making); + CPPUNIT_TEST(testMp3Making); + CPPUNIT_TEST(testOggMaking); + CPPUNIT_TEST(testFlacMaking); CPPUNIT_TEST(testMkvMaking); +#endif CPPUNIT_TEST_SUITE_END(); public: @@ -99,15 +101,17 @@ public: void removeAllTags(); void testMkvParsing(); - void testMkvMaking(); void testMp4Parsing(); - void testMp4Making(); void testMp3Parsing(); - void testMp3Making(); void testOggParsing(); - void testOggMaking(); void testFlacParsing(); +#ifdef PLATFORM_UNIX + void testMkvMaking(); + void testMp4Making(); + void testMp3Making(); + void testOggMaking(); void testFlacMaking(); +#endif private: MediaFileInfo m_fileInfo; @@ -1238,6 +1242,7 @@ void OverallTests::testMkvParsing() parseFile(TestUtilities::testFilePath("matroska_wave1/test8.mkv"), &OverallTests::checkMkvTestfile8); } +#ifdef PLATFORM_UNIX /*! * \brief Tests the Matroska maker via MediaFileInfo. * \remarks Relies on the parser to check results. @@ -1318,6 +1323,7 @@ void OverallTests::testMkvMaking() makeFile(TestUtilities::workingCopyPath("matroska_wave1/test8.mkv"), modifyRoutine, &OverallTests::checkMkvTestfile8); } } +#endif /*! * \brief Tests the MP4 parser via MediaFileInfo. @@ -1334,6 +1340,7 @@ void OverallTests::testMp4Parsing() parseFile(TestUtilities::testFilePath("mtx-test-data/aac/he-aacv2-ps.m4a"), &OverallTests::checkMp4Testfile5); } +#ifdef PLATFORM_UNIX /*! * \brief Tests the MP4 maker via MediaFileInfo. * \remarks Relies on the parser to check results. @@ -1394,6 +1401,7 @@ void OverallTests::testMp4Making() makeFile(TestUtilities::workingCopyPath("mtx-test-data/aac/he-aacv2-ps.m4a"), modifyRoutine, &OverallTests::checkMp4Testfile5); } } +#endif /*! * \brief Tests the MP3 parser via MediaFileInfo. @@ -1406,6 +1414,7 @@ void OverallTests::testMp3Parsing() parseFile(TestUtilities::testFilePath("mtx-test-data/mp3/id3-tag-and-xing-header.mp3"), &OverallTests::checkMp3Testfile1); } +#ifdef PLATFORM_UNIX /*! * \brief Tests the MP3 maker via MediaFileInfo. * \remarks Relies on the parser to check results. @@ -1455,6 +1464,7 @@ void OverallTests::testMp3Making() makeFile(TestUtilities::workingCopyPath("mtx-test-data/mp3/id3-tag-and-xing-header.mp3"), modifyRoutine, &OverallTests::checkMp3Testfile1); } } +#endif /*! * \brief Tests the Ogg parser via MediaFileInfo. @@ -1469,6 +1479,7 @@ void OverallTests::testOggParsing() parseFile(TestUtilities::testFilePath("mtx-test-data/opus/v-opus.ogg"), &OverallTests::checkOggTestfile2); } +#ifdef PLATFORM_UNIX /*! * \brief Tests the Ogg maker via MediaFileInfo. * \remarks @@ -1502,6 +1513,7 @@ void OverallTests::testOggMaking() makeFile(TestUtilities::workingCopyPath("mtx-test-data/opus/v-opus.ogg"), modifyRoutine, &OverallTests::checkOggTestfile2); } } +#endif /*! * \brief Tests the FLAC parser via MediaFileInfo. @@ -1515,6 +1527,7 @@ void OverallTests::testFlacParsing() parseFile(TestUtilities::testFilePath("flac/test.ogg"), &OverallTests::checkFlacTestfile2); } +#ifdef PLATFORM_UNIX /*! * \brief Tests the FLAC maker via MediaFileInfo. * \remarks Relies on the parser to check results. @@ -1545,3 +1558,4 @@ void OverallTests::testFlacMaking() makeFile(TestUtilities::workingCopyPath("flac/test.ogg"), modifyRoutine, &OverallTests::checkFlacTestfile2); } } +#endif