From b698c73690b7a858335763bcae0fe3229925e40b Mon Sep 17 00:00:00 2001 From: Ryan Lucchese Date: Tue, 30 Jun 2015 12:18:12 -0600 Subject: [PATCH 01/69] return const correct reverse iterator to prevent Solaris compiler from choking on const conversion --- taglib/toolkit/tbytevector.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 45ad0317..e9175d64 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -746,7 +746,7 @@ ByteVector::ReverseIterator ByteVector::rbegin() ByteVector::ConstReverseIterator ByteVector::rbegin() const { - return d->data->data.rbegin() + (d->data->data.size() - (d->offset + d->length)); + return ConstReverseIterator(static_cast(&*(d->data->data.rbegin() + (d->data->data.size() - (d->offset + d->length))))); } ByteVector::ReverseIterator ByteVector::rend() @@ -757,7 +757,7 @@ ByteVector::ReverseIterator ByteVector::rend() ByteVector::ConstReverseIterator ByteVector::rend() const { - return d->data->data.rbegin() + (d->data->data.size() - d->offset); + return ConstReverseIterator(static_cast(&*(d->data->data.rbegin() + (d->data->data.size() - d->offset)))); } bool ByteVector::isNull() const From a3dccdc7a3a7c5121adf96aa91d58df58343361e Mon Sep 17 00:00:00 2001 From: Ryan Lucchese Date: Tue, 30 Jun 2015 12:46:25 -0600 Subject: [PATCH 02/69] added sun compiler version check before changing constness of ConstReverseIterator in ByteVector --- taglib/toolkit/tbytevector.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index e9175d64..ac32af71 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -746,7 +746,11 @@ ByteVector::ReverseIterator ByteVector::rbegin() ByteVector::ConstReverseIterator ByteVector::rbegin() const { +#if __SUNPRO_CC >= 0x5130 return ConstReverseIterator(static_cast(&*(d->data->data.rbegin() + (d->data->data.size() - (d->offset + d->length))))); +#else + return d->data->data.rbegin() + (d->data->data.size() - (d->offset + d->length)); +#endif } ByteVector::ReverseIterator ByteVector::rend() @@ -757,7 +761,11 @@ ByteVector::ReverseIterator ByteVector::rend() ByteVector::ConstReverseIterator ByteVector::rend() const { +#if __SUNPRO_CC >= 0x5130 return ConstReverseIterator(static_cast(&*(d->data->data.rbegin() + (d->data->data.size() - d->offset)))); +#else + return d->data->data.rbegin() + (d->data->data.size() - d->offset); +#endif } bool ByteVector::isNull() const From 00b2f6a3862f4557ac9ece31202d1835ae49f818 Mon Sep 17 00:00:00 2001 From: Ryan Lucchese Date: Tue, 30 Jun 2015 12:50:03 -0600 Subject: [PATCH 03/69] check SUNPRO_CC version before defining WANT_CLASS_INSTANTIATION_OF_MAP in apetag.cpp - causes problems on newer Solaris Studio compiler --- taglib/ape/apetag.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/ape/apetag.cpp b/taglib/ape/apetag.cpp index e0c2a24e..7a2b6dda 100644 --- a/taglib/ape/apetag.cpp +++ b/taglib/ape/apetag.cpp @@ -23,7 +23,7 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ -#ifdef __SUNPRO_CC +#if __SUNPRO_CC < 0x5130 // Sun Studio finds multiple specializations of Map because // it considers specializations with and without class types // to be different; this define forces Map to use only the From 6d0712c8dfbf2000d3374d3f157dcc7b54a4c0ac Mon Sep 17 00:00:00 2001 From: Ryan Lucchese Date: Tue, 30 Jun 2015 13:23:25 -0600 Subject: [PATCH 04/69] changed SUNPRO_CC version check to first check if SUNPRO_CC is defined --- taglib/ape/apetag.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/ape/apetag.cpp b/taglib/ape/apetag.cpp index 7a2b6dda..db817ac9 100644 --- a/taglib/ape/apetag.cpp +++ b/taglib/ape/apetag.cpp @@ -23,7 +23,7 @@ * http://www.mozilla.org/MPL/ * ***************************************************************************/ -#if __SUNPRO_CC < 0x5130 +#if defined(__SUNPRO_CC) && (__SUNPRO_CC < 0x5130) // Sun Studio finds multiple specializations of Map because // it considers specializations with and without class types // to be different; this define forces Map to use only the From d8e8ec69fe471ac034dfc24d4576bd32081bae90 Mon Sep 17 00:00:00 2001 From: Ryan Lucchese Date: Tue, 30 Jun 2015 13:41:09 -0600 Subject: [PATCH 05/69] changed SUNPRO_CC version check to first check if SUNPRO_CC is defined --- taglib/toolkit/tbytevector.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index ac32af71..4c17a739 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -746,7 +746,7 @@ ByteVector::ReverseIterator ByteVector::rbegin() ByteVector::ConstReverseIterator ByteVector::rbegin() const { -#if __SUNPRO_CC >= 0x5130 +#if defined(__SUNPRO_CC) && (__SUNPRO_CC >= 0x5130) return ConstReverseIterator(static_cast(&*(d->data->data.rbegin() + (d->data->data.size() - (d->offset + d->length))))); #else return d->data->data.rbegin() + (d->data->data.size() - (d->offset + d->length)); @@ -761,7 +761,7 @@ ByteVector::ReverseIterator ByteVector::rend() ByteVector::ConstReverseIterator ByteVector::rend() const { -#if __SUNPRO_CC >= 0x5130 +#if defined(__SUNPRO_CC) && (__SUNPRO_CC >= 0x5130) return ConstReverseIterator(static_cast(&*(d->data->data.rbegin() + (d->data->data.size() - d->offset)))); #else return d->data->data.rbegin() + (d->data->data.size() - d->offset); From e8c1a1173062dca6c08cd152e676295c66ba0b6a Mon Sep 17 00:00:00 2001 From: Ryan Lucchese Date: Wed, 1 Jul 2015 15:47:21 -0600 Subject: [PATCH 06/69] better const correctness for ByteVector::rbegin() const and ByteVector::rend() const --- taglib/toolkit/tbytevector.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 4c17a739..fb561e25 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -746,11 +746,8 @@ ByteVector::ReverseIterator ByteVector::rbegin() ByteVector::ConstReverseIterator ByteVector::rbegin() const { -#if defined(__SUNPRO_CC) && (__SUNPRO_CC >= 0x5130) - return ConstReverseIterator(static_cast(&*(d->data->data.rbegin() + (d->data->data.size() - (d->offset + d->length))))); -#else - return d->data->data.rbegin() + (d->data->data.size() - (d->offset + d->length)); -#endif + const std::vector &v = d->data->data; + return v.rbegin() + (v.size() - (d->offset + d->length)); } ByteVector::ReverseIterator ByteVector::rend() @@ -761,11 +758,8 @@ ByteVector::ReverseIterator ByteVector::rend() ByteVector::ConstReverseIterator ByteVector::rend() const { -#if defined(__SUNPRO_CC) && (__SUNPRO_CC >= 0x5130) - return ConstReverseIterator(static_cast(&*(d->data->data.rbegin() + (d->data->data.size() - d->offset)))); -#else - return d->data->data.rbegin() + (d->data->data.size() - d->offset); -#endif + const std::vector &v = d->data->data; + return v.rbegin() + (v.size() - d->offset); } bool ByteVector::isNull() const From 04bee3faeca7765b4ab8b87ab8fe0903f8c4ece4 Mon Sep 17 00:00:00 2001 From: Ryan Lucchese Date: Mon, 6 Jul 2015 16:25:57 -0600 Subject: [PATCH 07/69] added comments explaining ByteVector::rbegin() const and ByteVector::rend() const --- taglib/toolkit/tbytevector.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index fb561e25..7840d1a2 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -746,6 +746,7 @@ ByteVector::ReverseIterator ByteVector::rbegin() ByteVector::ConstReverseIterator ByteVector::rbegin() const { + // we need a const reference to the data vector so we can ensure the const version of rbegin() is called const std::vector &v = d->data->data; return v.rbegin() + (v.size() - (d->offset + d->length)); } @@ -758,6 +759,7 @@ ByteVector::ReverseIterator ByteVector::rend() ByteVector::ConstReverseIterator ByteVector::rend() const { + // we need a const reference to the data vector so we can ensure the const version of rbegin() is called const std::vector &v = d->data->data; return v.rbegin() + (v.size() - d->offset); } From 29ed26528107f996c8b19d6184b55a516ffdb862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Tue, 25 Aug 2015 17:04:34 +0200 Subject: [PATCH 08/69] Skip the patch version if it's 0 --- CMakeLists.txt | 8 ++++++-- bindings/c/taglib_c.pc.cmake | 2 +- taglib-config.cmake | 2 +- taglib.pc.cmake | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 4bde0d0a..dbd2fcd3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -56,7 +56,11 @@ set(TAGLIB_LIB_MAJOR_VERSION "1") set(TAGLIB_LIB_MINOR_VERSION "10") set(TAGLIB_LIB_PATCH_VERSION "0") -set(TAGLIB_LIB_VERSION_STRING "${TAGLIB_LIB_MAJOR_VERSION}.${TAGLIB_LIB_MINOR_VERSION}.${TAGLIB_LIB_PATCH_VERSION}") +if("${TAGLIB_LIB_PATCH_VERSION}" EQUAL "0") + set(TAGLIB_LIB_VERSION_STRING "${TAGLIB_LIB_MAJOR_VERSION}.${TAGLIB_LIB_MINOR_VERSION}") +else() + set(TAGLIB_LIB_VERSION_STRING "${TAGLIB_LIB_MAJOR_VERSION}.${TAGLIB_LIB_MINOR_VERSION}.${TAGLIB_LIB_PATCH_VERSION}") +endif() # 1. If the library source code has changed at all since the last update, then increment revision. # 2. If any interfaces have been added, removed, or changed since the last update, increment current, and set revision to 0. @@ -130,4 +134,4 @@ configure_file( if (NOT TARGET uninstall) add_custom_target(uninstall COMMAND "${CMAKE_COMMAND}" -P "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake") -endif() \ No newline at end of file +endif() diff --git a/bindings/c/taglib_c.pc.cmake b/bindings/c/taglib_c.pc.cmake index 61764fc3..232f4f78 100644 --- a/bindings/c/taglib_c.pc.cmake +++ b/bindings/c/taglib_c.pc.cmake @@ -7,6 +7,6 @@ includedir=${INCLUDE_INSTALL_DIR} Name: TagLib C Bindings Description: Audio meta-data library (C bindings) Requires: taglib -Version: ${TAGLIB_LIB_MAJOR_VERSION}.${TAGLIB_LIB_MINOR_VERSION}.${TAGLIB_LIB_PATCH_VERSION} +Version: ${TAGLIB_LIB_VERSION_STRING} Libs: -L${LIB_INSTALL_DIR} -ltag_c Cflags: -I${INCLUDE_INSTALL_DIR}/taglib diff --git a/taglib-config.cmake b/taglib-config.cmake index 2c2d2dbc..2bc2811a 100644 --- a/taglib-config.cmake +++ b/taglib-config.cmake @@ -35,7 +35,7 @@ do flags="$flags -I$includedir/taglib" ;; --version) - echo ${TAGLIB_LIB_MAJOR_VERSION}.${TAGLIB_LIB_MINOR_VERSION}.${TAGLIB_LIB_PATCH_VERSION} + echo ${TAGLIB_LIB_VERSION_STRING} ;; --prefix) echo $prefix diff --git a/taglib.pc.cmake b/taglib.pc.cmake index 909b8fcf..5ee50aa5 100644 --- a/taglib.pc.cmake +++ b/taglib.pc.cmake @@ -6,6 +6,6 @@ includedir=${INCLUDE_INSTALL_DIR} Name: TagLib Description: Audio meta-data library Requires: -Version: ${TAGLIB_LIB_MAJOR_VERSION}.${TAGLIB_LIB_MINOR_VERSION}.${TAGLIB_LIB_PATCH_VERSION} +Version: ${TAGLIB_LIB_VERSION_STRING} Libs: -L${LIB_INSTALL_DIR} -ltag Cflags: -I${INCLUDE_INSTALL_DIR}/taglib From aedfeba66bdd7b83964cb5ab1d48f5f19b6746f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Tue, 25 Aug 2015 20:46:11 +0200 Subject: [PATCH 09/69] Missed the full version string in taglib-config.cmd.cmake --- taglib-config.cmd.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib-config.cmd.cmake b/taglib-config.cmd.cmake index bef752ed..bbf3cf84 100644 --- a/taglib-config.cmd.cmake +++ b/taglib-config.cmd.cmake @@ -29,7 +29,7 @@ goto theend :doit if /i "%1#" == "--libs#" echo -L${LIB_INSTALL_DIR} -llibtag if /i "%1#" == "--cflags#" echo -I${INCLUDE_INSTALL_DIR}/taglib -if /i "%1#" == "--version#" echo ${TAGLIB_LIB_MAJOR_VERSION}.${TAGLIB_LIB_MINOR_VERSION}.${TAGLIB_LIB_PATCH_VERSION} +if /i "%1#" == "--version#" echo ${TAGLIB_LIB_VERSION_STRING} if /i "%1#" == "--prefix#" echo ${CMAKE_INSTALL_PREFIX} :theend From 021c53900225efe4adbbc8593c2f5c51aa4331f3 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 26 Aug 2015 08:14:31 +0900 Subject: [PATCH 10/69] Small cleanups in CMakeLists.txt. --- CMakeLists.txt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index dbd2fcd3..361dff18 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -38,15 +38,15 @@ set(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}/lib${LIB_SUFFIX}" CACHE PATH "The su set(INCLUDE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/include" CACHE PATH "The subdirectory to the header prefix" FORCE) if(APPLE) - option(BUILD_FRAMEWORK "Build an OS X framework" OFF) - set(FRAMEWORK_INSTALL_DIR "/Library/Frameworks" CACHE STRING "Directory to install frameworks to.") + option(BUILD_FRAMEWORK "Build an OS X framework" OFF) + set(FRAMEWORK_INSTALL_DIR "/Library/Frameworks" CACHE STRING "Directory to install frameworks to.") endif() if(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") endif() -if (MSVC AND ENABLE_STATIC_RUNTIME) +if(MSVC AND ENABLE_STATIC_RUNTIME) foreach(flag_var CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO) string(REGEX REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}") endforeach(flag_var) @@ -116,8 +116,8 @@ configure_file(taglib/taglib_config.h.cmake "${CMAKE_CURRENT_BINARY_DIR}/taglib_ add_subdirectory(taglib) add_subdirectory(bindings) if(BUILD_TESTS) - enable_testing() - add_subdirectory(tests) + enable_testing() + add_subdirectory(tests) endif(BUILD_TESTS) add_subdirectory(examples) @@ -131,7 +131,7 @@ configure_file( "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake" IMMEDIATE @ONLY) -if (NOT TARGET uninstall) +if(NOT TARGET uninstall) add_custom_target(uninstall COMMAND "${CMAKE_COMMAND}" -P "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake") endif() From fa6f33e552aa1d11a88f6432a54a34aa1d2dfa97 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 26 Aug 2015 15:05:34 +0900 Subject: [PATCH 11/69] Add and fix some comments in ConfigureChecks.cmake. --- ConfigureChecks.cmake | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/ConfigureChecks.cmake b/ConfigureChecks.cmake index 88980ea1..f0a45f5c 100644 --- a/ConfigureChecks.cmake +++ b/ConfigureChecks.cmake @@ -164,7 +164,7 @@ if(NOT HAVE_GCC_BYTESWAP) endif() endif() -# Determine whether your compiler supports some safer version of vsprintf. +# Check if your compiler supports some safer version of vsprintf. check_cxx_source_compiles(" #include @@ -190,17 +190,15 @@ if(NOT HAVE_VSNPRINTF) " HAVE_VSPRINTF_S) endif() -# Check for libz using the cmake supplied FindZLIB.cmake +# Check if zlib is installed. if(NOT ZLIB_SOURCE) find_package(ZLIB) - if(ZLIB_FOUND) - set(HAVE_ZLIB 1) - else() - set(HAVE_ZLIB 0) - endif() + set(HAVE_ZLIB ZLIB_FOUND) endif() +# Check if CppUnit is installed. + if(BUILD_TESTS) find_package(CppUnit) if(NOT CppUnit_FOUND) From b49e3e56202dc5a15f2c4eee6b2b8f978382d655 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 27 Aug 2015 11:29:40 +0900 Subject: [PATCH 12/69] Small cleanups in audioproperties.cpp. --- taglib/audioproperties.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/taglib/audioproperties.cpp b/taglib/audioproperties.cpp index 4598164c..217d9287 100644 --- a/taglib/audioproperties.cpp +++ b/taglib/audioproperties.cpp @@ -57,7 +57,7 @@ AudioProperties::~AudioProperties() } -int TagLib::AudioProperties::lengthInSeconds() const +int AudioProperties::lengthInSeconds() const { // This is an ugly workaround but we can't add a virtual function. // Should be virtual in taglib2. @@ -105,7 +105,7 @@ int TagLib::AudioProperties::lengthInSeconds() const return 0; } -int TagLib::AudioProperties::lengthInMilliseconds() const +int AudioProperties::lengthInMilliseconds() const { // This is an ugly workaround but we can't add a virtual function. // Should be virtual in taglib2. From bf45cfd84ae7a0db572a2ee96f42eed660b1eecc Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 28 Aug 2015 01:38:09 +0900 Subject: [PATCH 13/69] Check if QT_VERSION macro is defined. --- taglib/toolkit/tstring.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/toolkit/tstring.h b/taglib/toolkit/tstring.h index 5a705ada..59611ab0 100644 --- a/taglib/toolkit/tstring.h +++ b/taglib/toolkit/tstring.h @@ -42,7 +42,7 @@ * conversion happening in the background */ -#if QT_VERSION >= 0x040000 +#if defined(QT_VERSION) && (QT_VERSION >= 0x040000) #define QStringToTString(s) TagLib::String(s.toUtf8().data(), TagLib::String::UTF8) #else #define QStringToTString(s) TagLib::String(s.utf8().data(), TagLib::String::UTF8) From e178875b40e952fce8c81577310af194ec0322d8 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 28 Aug 2015 13:36:30 +0900 Subject: [PATCH 14/69] Mention that String::toWString()/toCWString() doesn't return UTF-32 string. --- taglib/toolkit/tstring.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/taglib/toolkit/tstring.h b/taglib/toolkit/tstring.h index 59611ab0..8b739882 100644 --- a/taglib/toolkit/tstring.h +++ b/taglib/toolkit/tstring.h @@ -197,7 +197,8 @@ namespace TagLib { /*! * Returns a deep copy of this String as a wstring. The returned string is - * encoded in UTF-16 (without BOM/CPU byte order). + * encoded in UTF-16 (without BOM/CPU byte order), not UTF-32 even if wchar_t + * is 32-bit wide. * * \see toCWString() */ @@ -226,7 +227,7 @@ namespace TagLib { /*! * Returns a standard C-style (null-terminated) wide character version of * this String. The returned string is encoded in UTF-16 (without BOM/CPU byte - * order). + * order), not UTF-32 even if wchar_t is 32-bit wide. * * The returned string is still owned by this String and should not be deleted * by the user. From 6d925da75e7dc84db22f3c7aee290136fa3878ea Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 27 May 2015 10:04:52 +0900 Subject: [PATCH 15/69] Skip both ID3v1 and APE tags when seeking the last MPEG frame. --- taglib/mpeg/mpegfile.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 43075cfc..9ae1dffa 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -469,7 +469,16 @@ long MPEG::File::firstFrameOffset() long MPEG::File::lastFrameOffset() { - return previousFrameOffset(hasID3v1Tag() ? d->ID3v1Location - 1 : length()); + long position; + + if(hasAPETag()) + position = d->APELocation - 1; + else if(hasID3v1Tag()) + position = d->ID3v1Location - 1; + else + position = length(); + + return previousFrameOffset(position); } bool MPEG::File::hasID3v1Tag() const From fbd3b71690cedd70b9175d55a1e089aaca748046 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sun, 30 Aug 2015 00:56:32 +0900 Subject: [PATCH 16/69] Hide an internal variable from a public header. --- taglib/toolkit/tstring.cpp | 23 ++++++++++++++--------- taglib/toolkit/tstring.h | 7 ------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/taglib/toolkit/tstring.cpp b/taglib/toolkit/tstring.cpp index 258e1fcf..be22c9c0 100644 --- a/taglib/toolkit/tstring.cpp +++ b/taglib/toolkit/tstring.cpp @@ -49,6 +49,14 @@ namespace { using namespace TagLib; + inline String::Type wcharByteOrder() + { + if(Utils::systemByteOrder() == Utils::LittleEndian) + return String::UTF16LE; + else + return String::UTF16BE; + } + inline size_t UTF16toUTF8( const wchar_t *src, size_t srcLength, char *dst, size_t dstLength) { @@ -183,9 +191,9 @@ String::String(const wstring &s, Type t) // This looks ugly but needed for the compatibility with TagLib1.8. // Should be removed in TabLib2.0. if (t == UTF16BE) - t = WCharByteOrder; + t = wcharByteOrder(); else if (t == UTF16LE) - t = (WCharByteOrder == UTF16LE ? UTF16BE : UTF16LE); + t = (wcharByteOrder() == UTF16LE ? UTF16BE : UTF16LE); copyFromUTF16(s.c_str(), s.length(), t); } @@ -201,9 +209,9 @@ String::String(const wchar_t *s, Type t) // This looks ugly but needed for the compatibility with TagLib1.8. // Should be removed in TabLib2.0. if (t == UTF16BE) - t = WCharByteOrder; + t = wcharByteOrder(); else if (t == UTF16LE) - t = (WCharByteOrder == UTF16LE ? UTF16BE : UTF16LE); + t = (wcharByteOrder() == UTF16LE ? UTF16BE : UTF16LE); copyFromUTF16(s, ::wcslen(s), t); } @@ -773,7 +781,7 @@ void String::copyFromUTF16(const wchar_t *s, size_t length, Type t) length--; } else - swap = (t != WCharByteOrder); + swap = (t != wcharByteOrder()); d->data.resize(length); if(length > 0) { @@ -813,7 +821,7 @@ void String::copyFromUTF16(const char *s, size_t length, Type t) length -= 2; } else - swap = (t != WCharByteOrder); + swap = (t != wcharByteOrder()); d->data.resize(length / 2); for(size_t i = 0; i < length / 2; ++i) { @@ -827,9 +835,6 @@ void String::copyFromUTF16(const char *s, size_t length, Type t) } } -const String::Type String::WCharByteOrder - = (Utils::systemByteOrder() == Utils::BigEndian) ? String::UTF16BE : String::UTF16LE; - } //////////////////////////////////////////////////////////////////////////////// diff --git a/taglib/toolkit/tstring.h b/taglib/toolkit/tstring.h index 8b739882..3fc20053 100644 --- a/taglib/toolkit/tstring.h +++ b/taglib/toolkit/tstring.h @@ -539,13 +539,6 @@ namespace TagLib { */ void copyFromUTF16(const char *s, size_t length, Type t); - /*! - * Indicates which byte order of UTF-16 is used to store strings internally. - * - * \note \e String::UTF16BE or \e String::UTF16LE - */ - static const Type WCharByteOrder; - class StringPrivate; StringPrivate *d; }; From 15a1505880d27f7d58f39a78946c05c6c6aa2bda Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 31 Aug 2015 01:48:29 +0900 Subject: [PATCH 17/69] A bit more accurate calculation of the AIFF audio length. --- taglib/riff/aiff/aiffproperties.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/riff/aiff/aiffproperties.cpp b/taglib/riff/aiff/aiffproperties.cpp index e345fb0c..f074fae3 100644 --- a/taglib/riff/aiff/aiffproperties.cpp +++ b/taglib/riff/aiff/aiffproperties.cpp @@ -179,7 +179,7 @@ void RIFF::AIFF::Properties::read(File *file) d->sampleRate = static_cast(sampleRate + 0.5); if(d->sampleFrames > 0 && d->sampleRate > 0) { - const double length = d->sampleFrames * 1000.0 / d->sampleRate; + const double length = d->sampleFrames * 1000.0 / sampleRate; d->length = static_cast(length + 0.5); d->bitrate = static_cast(streamLength * 8.0 / length + 0.5); } From 9666b64f28c912bae8a42b6b1f40dd15a7ef0bc6 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 25 Aug 2015 17:51:33 +0900 Subject: [PATCH 18/69] Fix an instance reference to a static data member. --- taglib/mpeg/id3v2/id3v2header.cpp | 34 ++++++++++++++----------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2header.cpp b/taglib/mpeg/id3v2/id3v2header.cpp index 10381053..6e567193 100644 --- a/taglib/mpeg/id3v2/id3v2header.cpp +++ b/taglib/mpeg/id3v2/id3v2header.cpp @@ -39,15 +39,14 @@ using namespace ID3v2; class Header::HeaderPrivate { public: - HeaderPrivate() : majorVersion(4), - revisionNumber(0), - unsynchronisation(false), - extendedHeader(false), - experimentalIndicator(false), - footerPresent(false), - tagSize(0) {} - - ~HeaderPrivate() {} + HeaderPrivate() : + majorVersion(4), + revisionNumber(0), + unsynchronisation(false), + extendedHeader(false), + experimentalIndicator(false), + footerPresent(false), + tagSize(0) {} uint majorVersion; uint revisionNumber; @@ -58,8 +57,6 @@ public: bool footerPresent; uint tagSize; - - static const uint size = 10; }; //////////////////////////////////////////////////////////////////////////////// @@ -68,7 +65,7 @@ public: TagLib::uint Header::size() { - return HeaderPrivate::size; + return 10; } ByteVector Header::fileIdentifier() @@ -80,14 +77,14 @@ ByteVector Header::fileIdentifier() // public members //////////////////////////////////////////////////////////////////////////////// -Header::Header() +Header::Header() : + d(new HeaderPrivate()) { - d = new HeaderPrivate; } -Header::Header(const ByteVector &data) +Header::Header(const ByteVector &data) : + d(new HeaderPrivate()) { - d = new HeaderPrivate; parse(data); } @@ -139,9 +136,9 @@ TagLib::uint Header::tagSize() const TagLib::uint Header::completeTagSize() const { if(d->footerPresent) - return d->tagSize + d->size + Footer::size(); + return d->tagSize + size() + Footer::size(); else - return d->tagSize + d->size; + return d->tagSize + size(); } void Header::setTagSize(uint s) @@ -199,7 +196,6 @@ void Header::parse(const ByteVector &data) if(data.size() < size()) return; - // do some sanity checking -- even in ID3v2.3.0 and less the tag size is a // synch-safe integer, so all bytes must be less than 128. If this is not // true then this is an invalid tag. From e2466a72f8510034a5a43f27d2aa47c888ec9bf6 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 31 Aug 2015 16:20:15 +0900 Subject: [PATCH 19/69] Revert "Skip both ID3v1 and APE tags when seeking the last MPEG frame." This reverts commit 6d925da75e7dc84db22f3c7aee290136fa3878ea. --- taglib/mpeg/mpegfile.cpp | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 9ae1dffa..43075cfc 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -469,16 +469,7 @@ long MPEG::File::firstFrameOffset() long MPEG::File::lastFrameOffset() { - long position; - - if(hasAPETag()) - position = d->APELocation - 1; - else if(hasID3v1Tag()) - position = d->ID3v1Location - 1; - else - position = length(); - - return previousFrameOffset(position); + return previousFrameOffset(hasID3v1Tag() ? d->ID3v1Location - 1 : length()); } bool MPEG::File::hasID3v1Tag() const From 12f59d774eb8db7b14fe69bbcf21bc463d659e6b Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 31 Aug 2015 16:20:41 +0900 Subject: [PATCH 20/69] Revert "Hide an internal variable from a public header." This reverts commit fbd3b71690cedd70b9175d55a1e089aaca748046. --- taglib/toolkit/tstring.cpp | 23 +++++++++-------------- taglib/toolkit/tstring.h | 7 +++++++ 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/taglib/toolkit/tstring.cpp b/taglib/toolkit/tstring.cpp index be22c9c0..258e1fcf 100644 --- a/taglib/toolkit/tstring.cpp +++ b/taglib/toolkit/tstring.cpp @@ -49,14 +49,6 @@ namespace { using namespace TagLib; - inline String::Type wcharByteOrder() - { - if(Utils::systemByteOrder() == Utils::LittleEndian) - return String::UTF16LE; - else - return String::UTF16BE; - } - inline size_t UTF16toUTF8( const wchar_t *src, size_t srcLength, char *dst, size_t dstLength) { @@ -191,9 +183,9 @@ String::String(const wstring &s, Type t) // This looks ugly but needed for the compatibility with TagLib1.8. // Should be removed in TabLib2.0. if (t == UTF16BE) - t = wcharByteOrder(); + t = WCharByteOrder; else if (t == UTF16LE) - t = (wcharByteOrder() == UTF16LE ? UTF16BE : UTF16LE); + t = (WCharByteOrder == UTF16LE ? UTF16BE : UTF16LE); copyFromUTF16(s.c_str(), s.length(), t); } @@ -209,9 +201,9 @@ String::String(const wchar_t *s, Type t) // This looks ugly but needed for the compatibility with TagLib1.8. // Should be removed in TabLib2.0. if (t == UTF16BE) - t = wcharByteOrder(); + t = WCharByteOrder; else if (t == UTF16LE) - t = (wcharByteOrder() == UTF16LE ? UTF16BE : UTF16LE); + t = (WCharByteOrder == UTF16LE ? UTF16BE : UTF16LE); copyFromUTF16(s, ::wcslen(s), t); } @@ -781,7 +773,7 @@ void String::copyFromUTF16(const wchar_t *s, size_t length, Type t) length--; } else - swap = (t != wcharByteOrder()); + swap = (t != WCharByteOrder); d->data.resize(length); if(length > 0) { @@ -821,7 +813,7 @@ void String::copyFromUTF16(const char *s, size_t length, Type t) length -= 2; } else - swap = (t != wcharByteOrder()); + swap = (t != WCharByteOrder); d->data.resize(length / 2); for(size_t i = 0; i < length / 2; ++i) { @@ -835,6 +827,9 @@ void String::copyFromUTF16(const char *s, size_t length, Type t) } } +const String::Type String::WCharByteOrder + = (Utils::systemByteOrder() == Utils::BigEndian) ? String::UTF16BE : String::UTF16LE; + } //////////////////////////////////////////////////////////////////////////////// diff --git a/taglib/toolkit/tstring.h b/taglib/toolkit/tstring.h index 3fc20053..8b739882 100644 --- a/taglib/toolkit/tstring.h +++ b/taglib/toolkit/tstring.h @@ -539,6 +539,13 @@ namespace TagLib { */ void copyFromUTF16(const char *s, size_t length, Type t); + /*! + * Indicates which byte order of UTF-16 is used to store strings internally. + * + * \note \e String::UTF16BE or \e String::UTF16LE + */ + static const Type WCharByteOrder; + class StringPrivate; StringPrivate *d; }; From 60558d6a4ae41e30abd23ab23167497a89142bb7 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 31 Aug 2015 16:20:58 +0900 Subject: [PATCH 21/69] Revert "A bit more accurate calculation of the AIFF audio length." This reverts commit 15a1505880d27f7d58f39a78946c05c6c6aa2bda. --- taglib/riff/aiff/aiffproperties.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/riff/aiff/aiffproperties.cpp b/taglib/riff/aiff/aiffproperties.cpp index f074fae3..e345fb0c 100644 --- a/taglib/riff/aiff/aiffproperties.cpp +++ b/taglib/riff/aiff/aiffproperties.cpp @@ -179,7 +179,7 @@ void RIFF::AIFF::Properties::read(File *file) d->sampleRate = static_cast(sampleRate + 0.5); if(d->sampleFrames > 0 && d->sampleRate > 0) { - const double length = d->sampleFrames * 1000.0 / sampleRate; + const double length = d->sampleFrames * 1000.0 / d->sampleRate; d->length = static_cast(length + 0.5); d->bitrate = static_cast(streamLength * 8.0 / length + 0.5); } From 3bb0b11bff14acd090e99465331f66bd247c8008 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 31 Aug 2015 16:21:26 +0900 Subject: [PATCH 22/69] Revert "Fix an instance reference to a static data member." This reverts commit 9666b64f28c912bae8a42b6b1f40dd15a7ef0bc6. --- taglib/mpeg/id3v2/id3v2header.cpp | 34 +++++++++++++++++-------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2header.cpp b/taglib/mpeg/id3v2/id3v2header.cpp index 6e567193..10381053 100644 --- a/taglib/mpeg/id3v2/id3v2header.cpp +++ b/taglib/mpeg/id3v2/id3v2header.cpp @@ -39,14 +39,15 @@ using namespace ID3v2; class Header::HeaderPrivate { public: - HeaderPrivate() : - majorVersion(4), - revisionNumber(0), - unsynchronisation(false), - extendedHeader(false), - experimentalIndicator(false), - footerPresent(false), - tagSize(0) {} + HeaderPrivate() : majorVersion(4), + revisionNumber(0), + unsynchronisation(false), + extendedHeader(false), + experimentalIndicator(false), + footerPresent(false), + tagSize(0) {} + + ~HeaderPrivate() {} uint majorVersion; uint revisionNumber; @@ -57,6 +58,8 @@ public: bool footerPresent; uint tagSize; + + static const uint size = 10; }; //////////////////////////////////////////////////////////////////////////////// @@ -65,7 +68,7 @@ public: TagLib::uint Header::size() { - return 10; + return HeaderPrivate::size; } ByteVector Header::fileIdentifier() @@ -77,14 +80,14 @@ ByteVector Header::fileIdentifier() // public members //////////////////////////////////////////////////////////////////////////////// -Header::Header() : - d(new HeaderPrivate()) +Header::Header() { + d = new HeaderPrivate; } -Header::Header(const ByteVector &data) : - d(new HeaderPrivate()) +Header::Header(const ByteVector &data) { + d = new HeaderPrivate; parse(data); } @@ -136,9 +139,9 @@ TagLib::uint Header::tagSize() const TagLib::uint Header::completeTagSize() const { if(d->footerPresent) - return d->tagSize + size() + Footer::size(); + return d->tagSize + d->size + Footer::size(); else - return d->tagSize + size(); + return d->tagSize + d->size; } void Header::setTagSize(uint s) @@ -196,6 +199,7 @@ void Header::parse(const ByteVector &data) if(data.size() < size()) return; + // do some sanity checking -- even in ID3v2.3.0 and less the tag size is a // synch-safe integer, so all bytes must be less than 128. If this is not // true then this is an invalid tag. From 030e17764043d1a955f9137c6af1ba102533c183 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 31 Aug 2015 16:28:12 +0900 Subject: [PATCH 23/69] Revert "Small cleanups in CMakeLists.txt." This reverts commit 021c53900225efe4adbbc8593c2f5c51aa4331f3. --- CMakeLists.txt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 361dff18..dbd2fcd3 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -38,15 +38,15 @@ set(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}/lib${LIB_SUFFIX}" CACHE PATH "The su set(INCLUDE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/include" CACHE PATH "The subdirectory to the header prefix" FORCE) if(APPLE) - option(BUILD_FRAMEWORK "Build an OS X framework" OFF) - set(FRAMEWORK_INSTALL_DIR "/Library/Frameworks" CACHE STRING "Directory to install frameworks to.") + option(BUILD_FRAMEWORK "Build an OS X framework" OFF) + set(FRAMEWORK_INSTALL_DIR "/Library/Frameworks" CACHE STRING "Directory to install frameworks to.") endif() if(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") endif() -if(MSVC AND ENABLE_STATIC_RUNTIME) +if (MSVC AND ENABLE_STATIC_RUNTIME) foreach(flag_var CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO) string(REGEX REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}") endforeach(flag_var) @@ -116,8 +116,8 @@ configure_file(taglib/taglib_config.h.cmake "${CMAKE_CURRENT_BINARY_DIR}/taglib_ add_subdirectory(taglib) add_subdirectory(bindings) if(BUILD_TESTS) - enable_testing() - add_subdirectory(tests) + enable_testing() + add_subdirectory(tests) endif(BUILD_TESTS) add_subdirectory(examples) @@ -131,7 +131,7 @@ configure_file( "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake" IMMEDIATE @ONLY) -if(NOT TARGET uninstall) +if (NOT TARGET uninstall) add_custom_target(uninstall COMMAND "${CMAKE_COMMAND}" -P "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake") endif() From 70f8fb1bae6b8244403cbc9a70ed5adeb8ead8b7 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 31 Aug 2015 16:28:24 +0900 Subject: [PATCH 24/69] Revert "Add and fix some comments in ConfigureChecks.cmake." This reverts commit fa6f33e552aa1d11a88f6432a54a34aa1d2dfa97. --- ConfigureChecks.cmake | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/ConfigureChecks.cmake b/ConfigureChecks.cmake index f0a45f5c..88980ea1 100644 --- a/ConfigureChecks.cmake +++ b/ConfigureChecks.cmake @@ -164,7 +164,7 @@ if(NOT HAVE_GCC_BYTESWAP) endif() endif() -# Check if your compiler supports some safer version of vsprintf. +# Determine whether your compiler supports some safer version of vsprintf. check_cxx_source_compiles(" #include @@ -190,15 +190,17 @@ if(NOT HAVE_VSNPRINTF) " HAVE_VSPRINTF_S) endif() -# Check if zlib is installed. +# Check for libz using the cmake supplied FindZLIB.cmake if(NOT ZLIB_SOURCE) find_package(ZLIB) - set(HAVE_ZLIB ZLIB_FOUND) + if(ZLIB_FOUND) + set(HAVE_ZLIB 1) + else() + set(HAVE_ZLIB 0) + endif() endif() -# Check if CppUnit is installed. - if(BUILD_TESTS) find_package(CppUnit) if(NOT CppUnit_FOUND) From 5ca4cd2f520e48d4decc597e572019cadca41d7a Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 31 Aug 2015 16:29:13 +0900 Subject: [PATCH 25/69] Revert "Small cleanups in audioproperties.cpp." This reverts commit b49e3e56202dc5a15f2c4eee6b2b8f978382d655. --- taglib/audioproperties.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/taglib/audioproperties.cpp b/taglib/audioproperties.cpp index 217d9287..4598164c 100644 --- a/taglib/audioproperties.cpp +++ b/taglib/audioproperties.cpp @@ -57,7 +57,7 @@ AudioProperties::~AudioProperties() } -int AudioProperties::lengthInSeconds() const +int TagLib::AudioProperties::lengthInSeconds() const { // This is an ugly workaround but we can't add a virtual function. // Should be virtual in taglib2. @@ -105,7 +105,7 @@ int AudioProperties::lengthInSeconds() const return 0; } -int AudioProperties::lengthInMilliseconds() const +int TagLib::AudioProperties::lengthInMilliseconds() const { // This is an ugly workaround but we can't add a virtual function. // Should be virtual in taglib2. From 9ad5bb1d62082396c3cec908508f0df11eb86c8e Mon Sep 17 00:00:00 2001 From: Hasso Tepper Date: Sat, 7 Jul 2012 01:25:27 +0300 Subject: [PATCH 26/69] Support for proprietary frames Apple iTunes uses to tag podcast files. --- taglib/mpeg/id3v2/id3v2frame.cpp | 12 ++++++++++-- taglib/mpeg/id3v2/id3v2framefactory.cpp | 11 ++++++++++- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index bee5375a..4313bcc6 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -117,7 +117,8 @@ Frame *Frame::createTextualFrame(const String &key, const StringList &values) // // check if the key is contained in the key<=>frameID mapping ByteVector frameID = keyToFrameID(key); if(!frameID.isNull()) { - if(frameID[0] == 'T'){ // text frame + // Apple proprietary WFED (Podcast URL) is in fact a text frame. + if(frameID[0] == 'T' || frameID == "WFED"){ // text frame TextIdentificationFrame *frame = new TextIdentificationFrame(frameID, String::UTF8); frame->setText(values); return frame; @@ -427,6 +428,12 @@ static const char *frameTranslation[][2] = { // Other frames { "COMM", "COMMENT" }, //{ "USLT", "LYRICS" }, handled specially + // Apple iTunes proprietary frames + { "PCST", "PODCAST" }, + { "TCAT", "PODCASTCATEGORY" }, + { "TDES", "PODCASTDESC" }, + { "TGID", "PODCASTID" }, + { "WFED", "PODCASTURL" }, }; static const TagLib::uint txxxFrameTranslationSize = 8; @@ -532,7 +539,8 @@ PropertyMap Frame::asProperties() const // workaround until this function is virtual if(id == "TXXX") return dynamic_cast< const UserTextIdentificationFrame* >(this)->asProperties(); - else if(id[0] == 'T') + // Apple proprietary WFED (Podcast URL) is in fact a text frame. + else if(id[0] == 'T' || id == "WFED") return dynamic_cast< const TextIdentificationFrame* >(this)->asProperties(); else if(id == "WXXX") return dynamic_cast< const UserUrlLinkFrame* >(this)->asProperties(); diff --git a/taglib/mpeg/id3v2/id3v2framefactory.cpp b/taglib/mpeg/id3v2/id3v2framefactory.cpp index f6a4aac9..3b40ae45 100644 --- a/taglib/mpeg/id3v2/id3v2framefactory.cpp +++ b/taglib/mpeg/id3v2/id3v2framefactory.cpp @@ -167,7 +167,8 @@ Frame *FrameFactory::createFrame(const ByteVector &origData, Header *tagHeader) // Text Identification (frames 4.2) - if(frameID.startsWith("T")) { + // Apple proprietary WFED (Podcast URL) is in fact a text frame. + if(frameID.startsWith("T") || frameID == "WFED") { TextIdentificationFrame *f = frameID != "TXXX" ? new TextIdentificationFrame(data, header) @@ -423,6 +424,14 @@ bool FrameFactory::updateFrame(Frame::Header *header) const convertFrame("WPB", "WPUB", header); convertFrame("WXX", "WXXX", header); + // Apple iTunes nonstandard frames + convertFrame("PCS", "PCST", header); + convertFrame("TCT", "TCAT", header); + convertFrame("TDR", "TDRL", header); + convertFrame("TDS", "TDES", header); + convertFrame("TID", "TGID", header); + convertFrame("WFD", "WFED", header); + break; } From 8e7504a66f3a3dbc526264a77e2a6845ac0014a0 Mon Sep 17 00:00:00 2001 From: Urs Fleisch Date: Mon, 7 Sep 2015 21:00:46 +0200 Subject: [PATCH 27/69] Add support for ID2v2 PCST frames (podcast). --- taglib/CMakeLists.txt | 2 + taglib/mpeg/id3v2/frames/podcastframe.cpp | 79 ++++++++++++++++++++++ taglib/mpeg/id3v2/frames/podcastframe.h | 80 +++++++++++++++++++++++ taglib/mpeg/id3v2/id3v2framefactory.cpp | 6 ++ 4 files changed, 167 insertions(+) create mode 100644 taglib/mpeg/id3v2/frames/podcastframe.cpp create mode 100644 taglib/mpeg/id3v2/frames/podcastframe.h diff --git a/taglib/CMakeLists.txt b/taglib/CMakeLists.txt index 73c1a2f8..31e2c49b 100644 --- a/taglib/CMakeLists.txt +++ b/taglib/CMakeLists.txt @@ -83,6 +83,7 @@ set(tag_HDRS mpeg/id3v2/frames/urllinkframe.h mpeg/id3v2/frames/chapterframe.h mpeg/id3v2/frames/tableofcontentsframe.h + mpeg/id3v2/frames/podcastframe.h ogg/oggfile.h ogg/oggpage.h ogg/oggpageheader.h @@ -177,6 +178,7 @@ set(frames_SRCS mpeg/id3v2/frames/urllinkframe.cpp mpeg/id3v2/frames/chapterframe.cpp mpeg/id3v2/frames/tableofcontentsframe.cpp + mpeg/id3v2/frames/podcastframe.cpp ) set(ogg_SRCS diff --git a/taglib/mpeg/id3v2/frames/podcastframe.cpp b/taglib/mpeg/id3v2/frames/podcastframe.cpp new file mode 100644 index 00000000..5115a7dd --- /dev/null +++ b/taglib/mpeg/id3v2/frames/podcastframe.cpp @@ -0,0 +1,79 @@ +/*************************************************************************** + copyright : (C) 2015 by Urs Fleisch + email : ufleisch@users.sourceforge.net + ***************************************************************************/ + +/*************************************************************************** + * This library is free software; you can redistribute it and/or modify * + * it under the terms of the GNU Lesser General Public License version * + * 2.1 as published by the Free Software Foundation. * + * * + * This library is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with this library; if not, write to the Free Software * + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA * + * 02110-1301 USA * + * * + * Alternatively, this file is available under the Mozilla Public * + * License Version 1.1. You may obtain a copy of the License at * + * http://www.mozilla.org/MPL/ * + ***************************************************************************/ + +#include "podcastframe.h" + +using namespace TagLib; +using namespace ID3v2; + +class PodcastFrame::PodcastFramePrivate +{ +public: + ByteVector fieldData; +}; + +//////////////////////////////////////////////////////////////////////////////// +// public members +//////////////////////////////////////////////////////////////////////////////// + +PodcastFrame::PodcastFrame() : Frame("PCST") +{ + d = new PodcastFramePrivate; + d->fieldData = ByteVector(4, '\0'); +} + +PodcastFrame::~PodcastFrame() +{ + delete d; +} + +String PodcastFrame::toString() const +{ + return String(); +} + +//////////////////////////////////////////////////////////////////////////////// +// protected members +//////////////////////////////////////////////////////////////////////////////// + +void PodcastFrame::parseFields(const ByteVector &data) +{ + d->fieldData = data; +} + +ByteVector PodcastFrame::renderFields() const +{ + return d->fieldData; +} + +//////////////////////////////////////////////////////////////////////////////// +// private members +//////////////////////////////////////////////////////////////////////////////// + +PodcastFrame::PodcastFrame(const ByteVector &data, Header *h) : Frame(h) +{ + d = new PodcastFramePrivate; + parseFields(fieldData(data)); +} diff --git a/taglib/mpeg/id3v2/frames/podcastframe.h b/taglib/mpeg/id3v2/frames/podcastframe.h new file mode 100644 index 00000000..7bbc2138 --- /dev/null +++ b/taglib/mpeg/id3v2/frames/podcastframe.h @@ -0,0 +1,80 @@ +/*************************************************************************** + copyright : (C) 2015 by Urs Fleisch + email : ufleisch@users.sourceforge.net + ***************************************************************************/ + +/*************************************************************************** + * This library is free software; you can redistribute it and/or modify * + * it under the terms of the GNU Lesser General Public License version * + * 2.1 as published by the Free Software Foundation. * + * * + * This library is distributed in the hope that it will be useful, but * + * WITHOUT ANY WARRANTY; without even the implied warranty of * + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * + * Lesser General Public License for more details. * + * * + * You should have received a copy of the GNU Lesser General Public * + * License along with this library; if not, write to the Free Software * + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA * + * 02110-1301 USA * + * * + * Alternatively, this file is available under the Mozilla Public * + * License Version 1.1. You may obtain a copy of the License at * + * http://www.mozilla.org/MPL/ * + ***************************************************************************/ + +#ifndef TAGLIB_PODCASTFRAME_H +#define TAGLIB_PODCASTFRAME_H + +#include "id3v2frame.h" +#include "taglib_export.h" + +namespace TagLib { + + namespace ID3v2 { + + //! ID3v2 podcast frame + /*! + * An implementation of ID3v2 podcast flag, a frame with four zero bytes. + */ + class TAGLIB_EXPORT PodcastFrame : public Frame + { + friend class FrameFactory; + + public: + /*! + * Construct a podcast frame. + */ + PodcastFrame(); + + /*! + * Destroys this PodcastFrame instance. + */ + virtual ~PodcastFrame(); + + /*! + * Returns a null string. + */ + virtual String toString() const; + + protected: + // Reimplementations. + + virtual void parseFields(const ByteVector &data); + virtual ByteVector renderFields() const; + + private: + /*! + * The constructor used by the FrameFactory. + */ + PodcastFrame(const ByteVector &data, Header *h); + PodcastFrame(const PodcastFrame &); + PodcastFrame &operator=(const PodcastFrame &); + + class PodcastFramePrivate; + PodcastFramePrivate *d; + }; + + } +} +#endif diff --git a/taglib/mpeg/id3v2/id3v2framefactory.cpp b/taglib/mpeg/id3v2/id3v2framefactory.cpp index 3b40ae45..bf4b0ee8 100644 --- a/taglib/mpeg/id3v2/id3v2framefactory.cpp +++ b/taglib/mpeg/id3v2/id3v2framefactory.cpp @@ -49,6 +49,7 @@ #include "frames/eventtimingcodesframe.h" #include "frames/chapterframe.h" #include "frames/tableofcontentsframe.h" +#include "frames/podcastframe.h" using namespace TagLib; using namespace ID3v2; @@ -288,6 +289,11 @@ Frame *FrameFactory::createFrame(const ByteVector &origData, Header *tagHeader) if(frameID == "CTOC") return new TableOfContentsFrame(tagHeader, data, header); + // Apple proprietary PCST (Podcast) + + if(frameID == "PCST") + return new PodcastFrame(data, header); + return new UnknownFrame(data, header); } From 0a9068780501887dd731c60e441139c5caefa92d Mon Sep 17 00:00:00 2001 From: Peter Bauer Date: Wed, 23 Sep 2015 11:35:58 +0200 Subject: [PATCH 28/69] add options R, I, D for replace/insert/delete of arbitrary tags --- examples/tagwriter.cpp | 46 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/examples/tagwriter.cpp b/examples/tagwriter.cpp index f2896d76..ed8b0d7a 100644 --- a/examples/tagwriter.cpp +++ b/examples/tagwriter.cpp @@ -23,6 +23,7 @@ */ #include +#include #include #include @@ -34,6 +35,7 @@ #include #include #include +#include using namespace std; @@ -65,11 +67,32 @@ void usage() cout << " -g " << endl; cout << " -y " << endl; cout << " -T " << endl; + cout << " -R " << endl; + cout << " -I " << endl; + cout << " -D " << endl; cout << endl; exit(1); } +void checkForRejectedProperties(const TagLib::PropertyMap &tags) +{ // stolen from tagreader.cpp + if(tags.size() > 0) { + unsigned int longest = 0; + for(TagLib::PropertyMap::ConstIterator i = tags.begin(); i != tags.end(); ++i) { + if(i->first.size() > longest) { + longest = i->first.size(); + } + } + cout << "-- rejected TAGs (properties) --" << endl; + for(TagLib::PropertyMap::ConstIterator i = tags.begin(); i != tags.end(); ++i) { + for(TagLib::StringList::ConstIterator j = i->second.begin(); j != i->second.end(); ++j) { + cout << left << std::setw(longest) << i->first << " - " << '"' << *j << '"' << endl; + } + } + } +} + int main(int argc, char *argv[]) { TagLib::List fileList; @@ -121,6 +144,29 @@ int main(int argc, char *argv[]) case 'T': t->setTrack(value.toInt()); break; + case 'R': + case 'I': + if(i + 2 < argc) { + TagLib::PropertyMap map = (*it).file()->properties (); + if(field == 'R') { + map.replace(value, TagLib::String(argv[i + 2])); + } + else { + map.insert(value, TagLib::String(argv[i + 2])); + } + ++i; + checkForRejectedProperties((*it).file()->setProperties(map)); + } + else { + usage(); + } + break; + case 'D': { + TagLib::PropertyMap map = (*it).file()->properties(); + map.erase(value); + checkForRejectedProperties((*it).file()->setProperties(map)); + break; + } default: usage(); break; From 29be00dc5938cfaf7cf93d287d75b7d405f27dc3 Mon Sep 17 00:00:00 2001 From: Erwin Jansen Date: Fri, 9 Oct 2015 22:11:27 -0700 Subject: [PATCH 29/69] Fixes access violation - Fixes access violation when setting empty stringlist on integer properties in mp4 tag - Add a unit test that validates the fix. --- taglib/mp4/mp4tag.cpp | 6 +++--- tests/test_mp4.cpp | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index abfdbb42..1c188720 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -917,7 +917,7 @@ PropertyMap MP4::Tag::setProperties(const PropertyMap &props) for(; it != props.end(); ++it) { if(reverseKeyMap.contains(it->first)) { String name = reverseKeyMap[it->first]; - if(it->first == "TRACKNUMBER" || it->first == "DISCNUMBER") { + if((it->first == "TRACKNUMBER" || it->first == "DISCNUMBER") && it->second.size() > 0) { int first = 0, second = 0; StringList parts = StringList::split(it->second.front(), "/"); if(parts.size() > 0) { @@ -928,11 +928,11 @@ PropertyMap MP4::Tag::setProperties(const PropertyMap &props) d->items[name] = MP4::Item(first, second); } } - else if(it->first == "BPM") { + else if(it->first == "BPM" && it->second.size() > 0) { int value = it->second.front().toInt(); d->items[name] = MP4::Item(value); } - else if(it->first == "COMPILATION") { + else if(it->first == "COMPILATION" && it->second.size() > 0) { bool value = (it->second.front().toInt() != 0); d->items[name] = MP4::Item(value); } diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index 78e1badf..6841c43f 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -305,6 +305,14 @@ public: CPPUNIT_ASSERT(f.tag()->contains("cpil")); CPPUNIT_ASSERT_EQUAL(false, f.tag()->item("cpil").toBool()); CPPUNIT_ASSERT_EQUAL(StringList("0"), tags["COMPILATION"]); + + // Empty properties do not result in access violations + // when converting integers + tags["TRACKNUMBER"] = StringList(); + tags["DISCNUMBER"] = StringList(); + tags["BPM"] = StringList(); + tags["COMPILATION"] = StringList(); + f.setProperties(tags); } void testFuzzedFile() From 9fad0b28a52685b761212ee2a7ee24c2538e3d4e Mon Sep 17 00:00:00 2001 From: garima-g Date: Thu, 5 Nov 2015 11:09:20 +0530 Subject: [PATCH 30/69] Add self-assignment check in operator= Method 'operator=' should check its argument with 'this' pointer. --- taglib/ape/apeitem.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/taglib/ape/apeitem.cpp b/taglib/ape/apeitem.cpp index 3490173a..49c3a665 100644 --- a/taglib/ape/apeitem.cpp +++ b/taglib/ape/apeitem.cpp @@ -86,8 +86,10 @@ APE::Item::~Item() Item &APE::Item::operator=(const Item &item) { - delete d; - d = new ItemPrivate(*item.d); + if(&item != this) { + delete d; + d = new ItemPrivate(*item.d); + } return *this; } From ccaf6502144ec787c0d8cd9fc16bbe903f25b37a Mon Sep 17 00:00:00 2001 From: garima-g Date: Thu, 5 Nov 2015 11:12:24 +0530 Subject: [PATCH 31/69] Add self-assignment check in operator= Method 'operator=' should check its argument with 'this' pointer. --- taglib/asf/asfattribute.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/taglib/asf/asfattribute.cpp b/taglib/asf/asfattribute.cpp index 116bfe21..7a40bea3 100644 --- a/taglib/asf/asfattribute.cpp +++ b/taglib/asf/asfattribute.cpp @@ -72,10 +72,12 @@ ASF::Attribute::Attribute(const ASF::Attribute &other) ASF::Attribute &ASF::Attribute::operator=(const ASF::Attribute &other) { - if(d->deref()) - delete d; - d = other.d; - d->ref(); + if(&other != this) { + if(d->deref()) + delete d; + d = other.d; + d->ref(); + } return *this; } From 998ebf4ce6bc7d512b1c172a5ba286c616abcb11 Mon Sep 17 00:00:00 2001 From: garima-g Date: Thu, 5 Nov 2015 11:16:34 +0530 Subject: [PATCH 32/69] Add self-assignment check in operator= Method 'operator=' should check its argument with 'this' pointer. --- taglib/mp4/mp4coverart.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/taglib/mp4/mp4coverart.cpp b/taglib/mp4/mp4coverart.cpp index 2746469d..f2152335 100644 --- a/taglib/mp4/mp4coverart.cpp +++ b/taglib/mp4/mp4coverart.cpp @@ -54,11 +54,12 @@ MP4::CoverArt::CoverArt(const CoverArt &item) : d(item.d) MP4::CoverArt & MP4::CoverArt::operator=(const CoverArt &item) { - if(d->deref()) { - delete d; + if(&item != this) { + if(d->deref()) + delete d; + d = item.d; + d->ref(); } - d = item.d; - d->ref(); return *this; } From 8b4a27beb409c5a81f956b48c248c4fad0b258a3 Mon Sep 17 00:00:00 2001 From: garima-g Date: Thu, 5 Nov 2015 11:19:44 +0530 Subject: [PATCH 33/69] Add self-assignment check in operator= Method 'operator=' should check its argument with 'this' pointer. --- taglib/mp4/mp4item.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/taglib/mp4/mp4item.cpp b/taglib/mp4/mp4item.cpp index 671f26b4..aa59feda 100644 --- a/taglib/mp4/mp4item.cpp +++ b/taglib/mp4/mp4item.cpp @@ -64,11 +64,13 @@ MP4::Item::Item(const Item &item) : d(item.d) MP4::Item & MP4::Item::operator=(const Item &item) { - if(d->deref()) { - delete d; + if(&item != this) { + if(d->deref()) { + delete d; + } + d = item.d; + d->ref(); } - d = item.d; - d->ref(); return *this; } From 320d0f5ad74b306632fed1e342c41e6bb69688d1 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 6 Nov 2015 16:12:36 +0900 Subject: [PATCH 34/69] Use List::isEmpty() than size() > 0. Small revision of pokowaka's fix. isEmpty() is a little better than size() > 0, since std::list::empty() is guaranteed to be an O(1) operation. --- taglib/mp4/mp4tag.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index 1c188720..ce557c0f 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -917,7 +917,7 @@ PropertyMap MP4::Tag::setProperties(const PropertyMap &props) for(; it != props.end(); ++it) { if(reverseKeyMap.contains(it->first)) { String name = reverseKeyMap[it->first]; - if((it->first == "TRACKNUMBER" || it->first == "DISCNUMBER") && it->second.size() > 0) { + if((it->first == "TRACKNUMBER" || it->first == "DISCNUMBER") && !it->second.isEmpty()) { int first = 0, second = 0; StringList parts = StringList::split(it->second.front(), "/"); if(parts.size() > 0) { @@ -928,11 +928,11 @@ PropertyMap MP4::Tag::setProperties(const PropertyMap &props) d->items[name] = MP4::Item(first, second); } } - else if(it->first == "BPM" && it->second.size() > 0) { + else if(it->first == "BPM" && !it->second.isEmpty()) { int value = it->second.front().toInt(); d->items[name] = MP4::Item(value); } - else if(it->first == "COMPILATION" && it->second.size() > 0) { + else if(it->first == "COMPILATION" && !it->second.isEmpty()) { bool value = (it->second.front().toInt() != 0); d->items[name] = MP4::Item(value); } From 9f697fce8e287d6720cb37d5ee6e30eeccb9fb9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lalinsk=C3=BD?= Date: Wed, 11 Nov 2015 22:41:59 +0100 Subject: [PATCH 35/69] 1.10 --- CMakeLists.txt | 2 +- NEWS | 8 +++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index dbd2fcd3..f2455a79 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -67,7 +67,7 @@ endif() # 3. If any interfaces have been added since the last public release, then increment age. # 4. If any interfaces have been removed since the last public release, then set age to 0. set(TAGLIB_SOVERSION_CURRENT 16) -set(TAGLIB_SOVERSION_REVISION 0) +set(TAGLIB_SOVERSION_REVISION 1) set(TAGLIB_SOVERSION_AGE 15) math(EXPR TAGLIB_SOVERSION_MAJOR "${TAGLIB_SOVERSION_CURRENT} - ${TAGLIB_SOVERSION_AGE}") diff --git a/NEWS b/NEWS index 5db9da59..b5a27901 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,12 @@ -TagLib 1.10 (Aug 23, 2015) +TagLib 1.10 (Nov 11, 2015) ========================== +1.10: + + * Added new options to the tagwriter example. + * Fixed self-assignment operator in some types. + * Fixed extraction of MP4 tag keys with an empty list. + 1.10 BETA: * New API for the audio length in milliseconds. From 94ff9124c7ca15fe1b91c7d525cdf511b2cc65d5 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 12 Nov 2015 13:52:46 +0900 Subject: [PATCH 36/69] Skip both ID3v1 and APE tags when seeking the last MPEG frame. --- taglib/mpeg/mpegfile.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/taglib/mpeg/mpegfile.cpp b/taglib/mpeg/mpegfile.cpp index 43075cfc..9ae1dffa 100644 --- a/taglib/mpeg/mpegfile.cpp +++ b/taglib/mpeg/mpegfile.cpp @@ -469,7 +469,16 @@ long MPEG::File::firstFrameOffset() long MPEG::File::lastFrameOffset() { - return previousFrameOffset(hasID3v1Tag() ? d->ID3v1Location - 1 : length()); + long position; + + if(hasAPETag()) + position = d->APELocation - 1; + else if(hasID3v1Tag()) + position = d->ID3v1Location - 1; + else + position = length(); + + return previousFrameOffset(position); } bool MPEG::File::hasID3v1Tag() const From 47813c5a7fbe990257b8f1511d098f63a1b27247 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 12 Nov 2015 14:10:09 +0900 Subject: [PATCH 37/69] A bit more accurate calculation of the AIFF audio length. Actually, it's unlikely to improve the accuracy, but prevents a useless round-trip conversion between double and int. --- taglib/riff/aiff/aiffproperties.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/riff/aiff/aiffproperties.cpp b/taglib/riff/aiff/aiffproperties.cpp index e345fb0c..f074fae3 100644 --- a/taglib/riff/aiff/aiffproperties.cpp +++ b/taglib/riff/aiff/aiffproperties.cpp @@ -179,7 +179,7 @@ void RIFF::AIFF::Properties::read(File *file) d->sampleRate = static_cast(sampleRate + 0.5); if(d->sampleFrames > 0 && d->sampleRate > 0) { - const double length = d->sampleFrames * 1000.0 / d->sampleRate; + const double length = d->sampleFrames * 1000.0 / sampleRate; d->length = static_cast(length + 0.5); d->bitrate = static_cast(streamLength * 8.0 / length + 0.5); } From 1d379fdb2f8703f94106001656c60db57d259700 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 12 Nov 2015 14:24:04 +0900 Subject: [PATCH 38/69] Small cleanups in audioproperties.cpp. --- taglib/audioproperties.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/taglib/audioproperties.cpp b/taglib/audioproperties.cpp index 4598164c..217d9287 100644 --- a/taglib/audioproperties.cpp +++ b/taglib/audioproperties.cpp @@ -57,7 +57,7 @@ AudioProperties::~AudioProperties() } -int TagLib::AudioProperties::lengthInSeconds() const +int AudioProperties::lengthInSeconds() const { // This is an ugly workaround but we can't add a virtual function. // Should be virtual in taglib2. @@ -105,7 +105,7 @@ int TagLib::AudioProperties::lengthInSeconds() const return 0; } -int TagLib::AudioProperties::lengthInMilliseconds() const +int AudioProperties::lengthInMilliseconds() const { // This is an ugly workaround but we can't add a virtual function. // Should be virtual in taglib2. From c66e6b27d9658aa1a1a5984ec3caf2420bcedf29 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 12 Nov 2015 14:41:15 +0900 Subject: [PATCH 39/69] Small cleanups in CMakeLists.txt. --- CMakeLists.txt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f2455a79..88674bb9 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -38,15 +38,15 @@ set(LIB_INSTALL_DIR "${EXEC_INSTALL_PREFIX}/lib${LIB_SUFFIX}" CACHE PATH "The su set(INCLUDE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/include" CACHE PATH "The subdirectory to the header prefix" FORCE) if(APPLE) - option(BUILD_FRAMEWORK "Build an OS X framework" OFF) - set(FRAMEWORK_INSTALL_DIR "/Library/Frameworks" CACHE STRING "Directory to install frameworks to.") + option(BUILD_FRAMEWORK "Build an OS X framework" OFF) + set(FRAMEWORK_INSTALL_DIR "/Library/Frameworks" CACHE STRING "Directory to install frameworks to.") endif() if(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall") endif() -if (MSVC AND ENABLE_STATIC_RUNTIME) +if(MSVC AND ENABLE_STATIC_RUNTIME) foreach(flag_var CMAKE_CXX_FLAGS CMAKE_CXX_FLAGS_DEBUG CMAKE_CXX_FLAGS_RELEASE CMAKE_CXX_FLAGS_MINSIZEREL CMAKE_CXX_FLAGS_RELWITHDEBINFO) string(REGEX REPLACE "/MD" "/MT" ${flag_var} "${${flag_var}}") endforeach(flag_var) @@ -116,8 +116,8 @@ configure_file(taglib/taglib_config.h.cmake "${CMAKE_CURRENT_BINARY_DIR}/taglib_ add_subdirectory(taglib) add_subdirectory(bindings) if(BUILD_TESTS) - enable_testing() - add_subdirectory(tests) + enable_testing() + add_subdirectory(tests) endif(BUILD_TESTS) add_subdirectory(examples) @@ -131,7 +131,7 @@ configure_file( "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake" IMMEDIATE @ONLY) -if (NOT TARGET uninstall) +if(NOT TARGET uninstall) add_custom_target(uninstall COMMAND "${CMAKE_COMMAND}" -P "${CMAKE_CURRENT_BINARY_DIR}/cmake_uninstall.cmake") endif() From ec8e611909a8998e5b684cf5f22065c2d1ba9399 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 12 Nov 2015 14:48:24 +0900 Subject: [PATCH 40/69] Fix an instance reference to a static data member. --- taglib/mpeg/id3v2/id3v2header.cpp | 34 ++++++++++++++----------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2header.cpp b/taglib/mpeg/id3v2/id3v2header.cpp index 10381053..6e567193 100644 --- a/taglib/mpeg/id3v2/id3v2header.cpp +++ b/taglib/mpeg/id3v2/id3v2header.cpp @@ -39,15 +39,14 @@ using namespace ID3v2; class Header::HeaderPrivate { public: - HeaderPrivate() : majorVersion(4), - revisionNumber(0), - unsynchronisation(false), - extendedHeader(false), - experimentalIndicator(false), - footerPresent(false), - tagSize(0) {} - - ~HeaderPrivate() {} + HeaderPrivate() : + majorVersion(4), + revisionNumber(0), + unsynchronisation(false), + extendedHeader(false), + experimentalIndicator(false), + footerPresent(false), + tagSize(0) {} uint majorVersion; uint revisionNumber; @@ -58,8 +57,6 @@ public: bool footerPresent; uint tagSize; - - static const uint size = 10; }; //////////////////////////////////////////////////////////////////////////////// @@ -68,7 +65,7 @@ public: TagLib::uint Header::size() { - return HeaderPrivate::size; + return 10; } ByteVector Header::fileIdentifier() @@ -80,14 +77,14 @@ ByteVector Header::fileIdentifier() // public members //////////////////////////////////////////////////////////////////////////////// -Header::Header() +Header::Header() : + d(new HeaderPrivate()) { - d = new HeaderPrivate; } -Header::Header(const ByteVector &data) +Header::Header(const ByteVector &data) : + d(new HeaderPrivate()) { - d = new HeaderPrivate; parse(data); } @@ -139,9 +136,9 @@ TagLib::uint Header::tagSize() const TagLib::uint Header::completeTagSize() const { if(d->footerPresent) - return d->tagSize + d->size + Footer::size(); + return d->tagSize + size() + Footer::size(); else - return d->tagSize + d->size; + return d->tagSize + size(); } void Header::setTagSize(uint s) @@ -199,7 +196,6 @@ void Header::parse(const ByteVector &data) if(data.size() < size()) return; - // do some sanity checking -- even in ID3v2.3.0 and less the tag size is a // synch-safe integer, so all bytes must be less than 128. If this is not // true then this is an invalid tag. From 86c7e905ba7872fa3a4b3e96b0d8b6ca4fa35e56 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 13 Nov 2015 10:06:01 +0900 Subject: [PATCH 41/69] Silence some MSVC security warnings by replacing strdup() with _strdup(). Backported from taglib2. --- ConfigureChecks.cmake | 10 ++++++++ bindings/c/tag_c.cpp | 53 ++++++++++++++++++++++++++++++++----------- config.h.cmake | 3 +++ 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/ConfigureChecks.cmake b/ConfigureChecks.cmake index 88980ea1..5c740756 100644 --- a/ConfigureChecks.cmake +++ b/ConfigureChecks.cmake @@ -190,6 +190,16 @@ if(NOT HAVE_VSNPRINTF) " HAVE_VSPRINTF_S) endif() +# Determine whether your compiler supports ISO _strdup. + +check_cxx_source_compiles(" + #include + int main() { + _strdup(0); + return 0; +} +" HAVE_ISO_STRDUP) + # Check for libz using the cmake supplied FindZLIB.cmake if(NOT ZLIB_SOURCE) diff --git a/bindings/c/tag_c.cpp b/bindings/c/tag_c.cpp index 6e507b19..41dd15eb 100644 --- a/bindings/c/tag_c.cpp +++ b/bindings/c/tag_c.cpp @@ -19,6 +19,10 @@ * USA * ***************************************************************************/ +#ifdef HAVE_CONFIG_H +# include +#endif + #include #include #include @@ -40,9 +44,32 @@ using namespace TagLib; -static List strings; -static bool unicodeStrings = true; -static bool stringManagementEnabled = true; +namespace +{ + List strings; + bool unicodeStrings = true; + bool stringManagementEnabled = true; + + inline char *stringToCharArray(const String &s) + { + const std::string str = s.to8Bit(unicodeStrings); + +#ifdef HAVE_ISO_STRDUP + + return ::_strdup(str.c_str()); + +#else + + return ::strdup(str.c_str()); + +#endif + } + + inline String charArrayToString(const char *s) + { + return String(s, unicodeStrings ? String::UTF8 : String::Latin1); + } +} void taglib_set_strings_unicode(BOOL unicode) { @@ -132,7 +159,7 @@ BOOL taglib_file_save(TagLib_File *file) char *taglib_tag_title(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = ::strdup(t->title().toCString(unicodeStrings)); + char *s = stringToCharArray(t->title().toCString(unicodeStrings)); if(stringManagementEnabled) strings.append(s); return s; @@ -141,7 +168,7 @@ char *taglib_tag_title(const TagLib_Tag *tag) char *taglib_tag_artist(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = ::strdup(t->artist().toCString(unicodeStrings)); + char *s = stringToCharArray(t->artist().toCString(unicodeStrings)); if(stringManagementEnabled) strings.append(s); return s; @@ -150,7 +177,7 @@ char *taglib_tag_artist(const TagLib_Tag *tag) char *taglib_tag_album(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = ::strdup(t->album().toCString(unicodeStrings)); + char *s = stringToCharArray(t->album().toCString(unicodeStrings)); if(stringManagementEnabled) strings.append(s); return s; @@ -159,7 +186,7 @@ char *taglib_tag_album(const TagLib_Tag *tag) char *taglib_tag_comment(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = ::strdup(t->comment().toCString(unicodeStrings)); + char *s = stringToCharArray(t->comment().toCString(unicodeStrings)); if(stringManagementEnabled) strings.append(s); return s; @@ -168,7 +195,7 @@ char *taglib_tag_comment(const TagLib_Tag *tag) char *taglib_tag_genre(const TagLib_Tag *tag) { const Tag *t = reinterpret_cast(tag); - char *s = ::strdup(t->genre().toCString(unicodeStrings)); + char *s = stringToCharArray(t->genre().toCString(unicodeStrings)); if(stringManagementEnabled) strings.append(s); return s; @@ -189,31 +216,31 @@ unsigned int taglib_tag_track(const TagLib_Tag *tag) void taglib_tag_set_title(TagLib_Tag *tag, const char *title) { Tag *t = reinterpret_cast(tag); - t->setTitle(String(title, unicodeStrings ? String::UTF8 : String::Latin1)); + t->setTitle(charArrayToString(title)); } void taglib_tag_set_artist(TagLib_Tag *tag, const char *artist) { Tag *t = reinterpret_cast(tag); - t->setArtist(String(artist, unicodeStrings ? String::UTF8 : String::Latin1)); + t->setArtist(charArrayToString(artist)); } void taglib_tag_set_album(TagLib_Tag *tag, const char *album) { Tag *t = reinterpret_cast(tag); - t->setAlbum(String(album, unicodeStrings ? String::UTF8 : String::Latin1)); + t->setAlbum(charArrayToString(album)); } void taglib_tag_set_comment(TagLib_Tag *tag, const char *comment) { Tag *t = reinterpret_cast(tag); - t->setComment(String(comment, unicodeStrings ? String::UTF8 : String::Latin1)); + t->setComment(charArrayToString(comment)); } void taglib_tag_set_genre(TagLib_Tag *tag, const char *genre) { Tag *t = reinterpret_cast(tag); - t->setGenre(String(genre, unicodeStrings ? String::UTF8 : String::Latin1)); + t->setGenre(charArrayToString(genre)); } void taglib_tag_set_year(TagLib_Tag *tag, unsigned int year) diff --git a/config.h.cmake b/config.h.cmake index ee0e67ac..12e713fc 100644 --- a/config.h.cmake +++ b/config.h.cmake @@ -19,6 +19,9 @@ #cmakedefine HAVE_VSNPRINTF 1 #cmakedefine HAVE_VSPRINTF_S 1 +/* Defined if your compiler supports ISO _strdup. */ +#cmakedefine HAVE_ISO_STRDUP 1 + /* Defined if you have libz */ #cmakedefine HAVE_ZLIB 1 From 6775cef651f01753efff221264112af7f11aa183 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 13 Nov 2015 10:58:23 +0900 Subject: [PATCH 42/69] Make use of the Boost Endian library for byte swapping. It's likely to be better at choosing the most efficient method than our CMake tests. --- ConfigureChecks.cmake | 68 ++++++++++++++++++++++++----------------- config.h.cmake | 1 + taglib/toolkit/tutils.h | 24 ++++++++++++--- 3 files changed, 60 insertions(+), 33 deletions(-) diff --git a/ConfigureChecks.cmake b/ConfigureChecks.cmake index 5c740756..3fa01bb1 100644 --- a/ConfigureChecks.cmake +++ b/ConfigureChecks.cmake @@ -108,57 +108,69 @@ endif() # Determine which kind of byte swap functions your compiler supports. check_cxx_source_compiles(" + #include int main() { - __builtin_bswap16(0); - __builtin_bswap32(0); - __builtin_bswap64(0); + boost::endian::endian_reverse(static_cast(1)); + boost::endian::endian_reverse(static_cast(1)); + boost::endian::endian_reverse(static_cast(1)); return 0; } -" HAVE_GCC_BYTESWAP) +" HAVE_BOOST_BYTESWAP) -if(NOT HAVE_GCC_BYTESWAP) +if(NOT HAVE_BOOST_BYTESWAP) check_cxx_source_compiles(" - #include int main() { - __bswap_16(0); - __bswap_32(0); - __bswap_64(0); + __builtin_bswap16(0); + __builtin_bswap32(0); + __builtin_bswap64(0); return 0; } - " HAVE_GLIBC_BYTESWAP) + " HAVE_GCC_BYTESWAP) - if(NOT HAVE_GLIBC_BYTESWAP) + if(NOT HAVE_GCC_BYTESWAP) check_cxx_source_compiles(" - #include + #include int main() { - _byteswap_ushort(0); - _byteswap_ulong(0); - _byteswap_uint64(0); + __bswap_16(0); + __bswap_32(0); + __bswap_64(0); return 0; } - " HAVE_MSC_BYTESWAP) + " HAVE_GLIBC_BYTESWAP) - if(NOT HAVE_MSC_BYTESWAP) + if(NOT HAVE_GLIBC_BYTESWAP) check_cxx_source_compiles(" - #include + #include int main() { - OSSwapInt16(0); - OSSwapInt32(0); - OSSwapInt64(0); + _byteswap_ushort(0); + _byteswap_ulong(0); + _byteswap_uint64(0); return 0; } - " HAVE_MAC_BYTESWAP) + " HAVE_MSC_BYTESWAP) - if(NOT HAVE_MAC_BYTESWAP) + if(NOT HAVE_MSC_BYTESWAP) check_cxx_source_compiles(" - #include + #include int main() { - swap16(0); - swap32(0); - swap64(0); + OSSwapInt16(0); + OSSwapInt32(0); + OSSwapInt64(0); return 0; } - " HAVE_OPENBSD_BYTESWAP) + " HAVE_MAC_BYTESWAP) + + if(NOT HAVE_MAC_BYTESWAP) + check_cxx_source_compiles(" + #include + int main() { + swap16(0); + swap32(0); + swap64(0); + return 0; + } + " HAVE_OPENBSD_BYTESWAP) + endif() endif() endif() endif() diff --git a/config.h.cmake b/config.h.cmake index 12e713fc..55affab6 100644 --- a/config.h.cmake +++ b/config.h.cmake @@ -1,6 +1,7 @@ /* config.h. Generated by cmake from config.h.cmake */ /* Defined if your compiler supports some byte swap functions */ +#cmakedefine HAVE_BOOST_BYTESWAP 1 #cmakedefine HAVE_GCC_BYTESWAP 1 #cmakedefine HAVE_GLIBC_BYTESWAP 1 #cmakedefine HAVE_MSC_BYTESWAP 1 diff --git a/taglib/toolkit/tutils.h b/taglib/toolkit/tutils.h index 82f1dd9a..5c204d85 100644 --- a/taglib/toolkit/tutils.h +++ b/taglib/toolkit/tutils.h @@ -31,10 +31,12 @@ #ifndef DO_NOT_DOCUMENT // tell Doxygen not to document this header #ifdef HAVE_CONFIG_H -#include +# include #endif -#if defined(HAVE_MSC_BYTESWAP) +#if defined(HAVE_BOOST_BYTESWAP) +# include +#elif defined(HAVE_MSC_BYTESWAP) # include #elif defined(HAVE_GLIBC_BYTESWAP) # include @@ -59,7 +61,11 @@ namespace TagLib */ inline ushort byteSwap(ushort x) { -#if defined(HAVE_GCC_BYTESWAP) +#if defined(HAVE_BOOST_BYTESWAP) + + return boost::endian::endian_reverse(x); + +#elif defined(HAVE_GCC_BYTESWAP) return __builtin_bswap16(x); @@ -91,7 +97,11 @@ namespace TagLib */ inline uint byteSwap(uint x) { -#if defined(HAVE_GCC_BYTESWAP) +#if defined(HAVE_BOOST_BYTESWAP) + + return boost::endian::endian_reverse(x); + +#elif defined(HAVE_GCC_BYTESWAP) return __builtin_bswap32(x); @@ -126,7 +136,11 @@ namespace TagLib */ inline ulonglong byteSwap(ulonglong x) { -#if defined(HAVE_GCC_BYTESWAP) +#if defined(HAVE_BOOST_BYTESWAP) + + return boost::endian::endian_reverse(x); + +#elif defined(HAVE_GCC_BYTESWAP) return __builtin_bswap64(x); From 762581fe3f48075cbaef094c9888481577f298ac Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 12 Jun 2015 15:12:58 +0900 Subject: [PATCH 43/69] Add a method to check if an MP4 file on disk actually has a tag. --- taglib/mp4/mp4file.cpp | 26 +++++++++++++++++++++----- taglib/mp4/mp4file.h | 5 +++++ tests/test_mp4.cpp | 4 ++++ 3 files changed, 30 insertions(+), 5 deletions(-) diff --git a/taglib/mp4/mp4file.cpp b/taglib/mp4/mp4file.cpp index 1fc1524f..566a5e32 100644 --- a/taglib/mp4/mp4file.cpp +++ b/taglib/mp4/mp4file.cpp @@ -34,7 +34,7 @@ using namespace TagLib; namespace { - bool checkValid(const MP4::AtomList &list) + inline bool checkValid(const MP4::AtomList &list) { for(MP4::AtomList::ConstIterator it = list.begin(); it != list.end(); ++it) { @@ -55,7 +55,8 @@ public: FilePrivate() : tag(0), atoms(0), - properties(0) {} + properties(0), + hasMP4Tag(false) {} ~FilePrivate() { @@ -67,6 +68,8 @@ public: MP4::Tag *tag; MP4::Atoms *atoms; MP4::Properties *properties; + + bool hasMP4Tag; }; MP4::File::File(FileName file, bool readProperties, AudioProperties::ReadStyle) : @@ -130,12 +133,15 @@ MP4::File::read(bool readProperties) } // must have a moov atom, otherwise consider it invalid - MP4::Atom *moov = d->atoms->find("moov"); - if(!moov) { + if(!d->atoms->find("moov")) { setValid(false); return; } + if(d->atoms->find("moov", "udta", "meta", "ilst")) { + d->hasMP4Tag = true; + } + d->tag = new Tag(this, d->atoms); if(readProperties) { d->properties = new Properties(this, d->atoms); @@ -155,6 +161,16 @@ MP4::File::save() return false; } - return d->tag->save(); + const bool success = d->tag->save(); + if(success) { + d->hasMP4Tag = true; + } + + return success; } +bool +MP4::File::hasMP4Tag() const +{ + return d->hasMP4Tag; +} diff --git a/taglib/mp4/mp4file.h b/taglib/mp4/mp4file.h index 791a0192..8c049301 100644 --- a/taglib/mp4/mp4file.h +++ b/taglib/mp4/mp4file.h @@ -117,6 +117,11 @@ namespace TagLib { */ bool save(); + /*! + * Returns whether or not the file on disk actually has an MP4 tag. + */ + bool hasMP4Tag() const; + private: void read(bool readProperties); diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index 6841c43f..ed93f043 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -69,6 +69,10 @@ public: CPPUNIT_ASSERT(!f.isValid()); MP4::File f2(TEST_FILE_PATH_C("has-tags.m4a")); CPPUNIT_ASSERT(f2.isValid()); + CPPUNIT_ASSERT(f2.hasMP4Tag()); + MP4::File f3(TEST_FILE_PATH_C("no-tags.m4a")); + CPPUNIT_ASSERT(f3.isValid()); + CPPUNIT_ASSERT(!f3.hasMP4Tag()); } void testIsEmpty() From c353a71ce5aff16ea7bb8ba18191f0fdbba92ed4 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 13 Nov 2015 11:23:27 +0900 Subject: [PATCH 44/69] Remove an unused private data member. --- taglib/mpeg/id3v2/id3v2tag.cpp | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index e31d1247..fd807b67 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -52,10 +52,15 @@ using namespace ID3v2; class ID3v2::Tag::TagPrivate { public: - TagPrivate() : file(0), tagOffset(-1), extendedHeader(0), footer(0), paddingSize(0) + TagPrivate() : + file(0), + tagOffset(-1), + extendedHeader(0), + footer(0) { frameList.setAutoDelete(true); } + ~TagPrivate() { delete extendedHeader; @@ -70,8 +75,6 @@ public: ExtendedHeader *extendedHeader; Footer *footer; - int paddingSize; - FrameListMap frameListMap; FrameList frameList; @@ -107,17 +110,17 @@ String Latin1StringHandler::parse(const ByteVector &data) const // public members //////////////////////////////////////////////////////////////////////////////// -ID3v2::Tag::Tag() : TagLib::Tag() +ID3v2::Tag::Tag() : + TagLib::Tag(), + d(new TagPrivate()) { - d = new TagPrivate; d->factory = FrameFactory::instance(); } ID3v2::Tag::Tag(File *file, long tagOffset, const FrameFactory *factory) : - TagLib::Tag() + TagLib::Tag(), + d(new TagPrivate()) { - d = new TagPrivate; - d->file = file; d->tagOffset = tagOffset; d->factory = factory; @@ -710,7 +713,6 @@ void ID3v2::Tag::parse(const ByteVector &origData) debug("Padding *and* a footer found. This is not allowed by the spec."); } - d->paddingSize = frameDataLength - frameDataPosition; break; } From 091ab9dee0fa9f961a88adf79e67825d4f292d9a Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 13 Nov 2015 11:35:37 +0900 Subject: [PATCH 45/69] Reduce memory reallocation when rendering an ID3v2 tag. Prevent an ID3v2 padding from being ridiculously large. --- taglib/mpeg/id3v2/id3v2tag.cpp | 45 ++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 16 deletions(-) diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index fd807b67..de8872f1 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -27,6 +27,8 @@ #include "config.h" #endif +#include + #include "tfile.h" #include "id3v2tag.h" @@ -86,7 +88,8 @@ const ID3v2::Latin1StringHandler *ID3v2::Tag::TagPrivate::stringHandler = &defau namespace { - const TagLib::uint DefaultPaddingSize = 1024; + const long MinPaddingSize = 1024; + const long MaxPaddingSize = 1024 * 1024; } //////////////////////////////////////////////////////////////////////////////// @@ -569,8 +572,6 @@ ByteVector ID3v2::Tag::render(int version) const // in ID3v2::Header::tagSize() -- includes the extended header, frames and // padding, but does not include the tag's header or footer. - ByteVector tagData; - if(version != 3 && version != 4) { debug("Unknown ID3v2 version, using ID3v2.4"); version = 4; @@ -578,7 +579,7 @@ ByteVector ID3v2::Tag::render(int version) const // TODO: Render the extended header. - // Loop through the frames rendering them and adding them to the tagData. + // Downgrade the frames that ID3v2.3 doesn't support. FrameList newFrames; newFrames.setAutoDelete(true); @@ -591,6 +592,12 @@ ByteVector ID3v2::Tag::render(int version) const downgradeFrames(&frameList, &newFrames); } + // Reserve a 10-byte blank space for an ID3v2 tag header. + + ByteVector tagData(Header::size(), '\0'); + + // Loop through the frames rendering them and adding them to the tagData. + for(FrameList::ConstIterator it = frameList.begin(); it != frameList.end(); it++) { (*it)->header()->setVersion(version); if((*it)->header()->frameID().size() != 4) { @@ -610,29 +617,35 @@ ByteVector ID3v2::Tag::render(int version) const } // Compute the amount of padding, and append that to tagData. + // TODO: Should be calculated in offset_t in taglib2. - uint paddingSize = DefaultPaddingSize; + long paddingSize = d->header.tagSize() - tagData.size(); - if(d->file && tagData.size() < d->header.tagSize()) { - paddingSize = d->header.tagSize() - tagData.size(); + if(paddingSize <= 0) { + paddingSize = MinPaddingSize; + } + else { + // Padding won't increase beyond 1% of the file size or 1MB. - // Padding won't increase beyond 1% of the file size. + long threshold = d->file ? d->file->length() / 100 : 0; + threshold = std::max(threshold, MinPaddingSize); + threshold = std::min(threshold, MaxPaddingSize); - if(paddingSize > DefaultPaddingSize) { - const uint threshold = d->file->length() / 100; // should be ulonglong in taglib2. - if(paddingSize > threshold) - paddingSize = DefaultPaddingSize; - } + if(paddingSize > threshold) + paddingSize = MinPaddingSize; } - tagData.append(ByteVector(paddingSize, '\0')); + tagData.resize(static_cast(tagData.size() + paddingSize), '\0'); // Set the version and data size. d->header.setMajorVersion(version); - d->header.setTagSize(tagData.size()); + d->header.setTagSize(tagData.size() - Header::size()); // TODO: This should eventually include d->footer->render(). - return d->header.render() + tagData; + const ByteVector headerData = d->header.render(); + std::copy(headerData.begin(), headerData.end(), tagData.begin()); + + return tagData; } Latin1StringHandler const *ID3v2::Tag::latin1StringHandler() From a25e1e9f90af82d3c925ef806cb76e35624a0f41 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Fri, 13 Nov 2015 11:44:12 +0900 Subject: [PATCH 46/69] Correct the ID3v2 padding size calculation. --- taglib/mpeg/id3v2/id3v2tag.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/mpeg/id3v2/id3v2tag.cpp b/taglib/mpeg/id3v2/id3v2tag.cpp index de8872f1..e41606db 100644 --- a/taglib/mpeg/id3v2/id3v2tag.cpp +++ b/taglib/mpeg/id3v2/id3v2tag.cpp @@ -619,7 +619,7 @@ ByteVector ID3v2::Tag::render(int version) const // Compute the amount of padding, and append that to tagData. // TODO: Should be calculated in offset_t in taglib2. - long paddingSize = d->header.tagSize() - tagData.size(); + long paddingSize = d->header.tagSize() - (tagData.size() - Header::size()); if(paddingSize <= 0) { paddingSize = MinPaddingSize; From 67f44071cdb4ac16401687753b675384a177b81e Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Sat, 14 Nov 2015 14:49:59 +0900 Subject: [PATCH 47/69] Fix the usage of boost::endian::endian_reverse(). --- taglib/toolkit/tutils.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/taglib/toolkit/tutils.h b/taglib/toolkit/tutils.h index 5c204d85..36e0afd3 100644 --- a/taglib/toolkit/tutils.h +++ b/taglib/toolkit/tutils.h @@ -63,7 +63,7 @@ namespace TagLib { #if defined(HAVE_BOOST_BYTESWAP) - return boost::endian::endian_reverse(x); + return boost::endian::endian_reverse(static_cast(x)); #elif defined(HAVE_GCC_BYTESWAP) @@ -99,7 +99,7 @@ namespace TagLib { #if defined(HAVE_BOOST_BYTESWAP) - return boost::endian::endian_reverse(x); + return boost::endian::endian_reverse(static_cast(x)); #elif defined(HAVE_GCC_BYTESWAP) @@ -138,7 +138,7 @@ namespace TagLib { #if defined(HAVE_BOOST_BYTESWAP) - return boost::endian::endian_reverse(x); + return boost::endian::endian_reverse(static_cast(x)); #elif defined(HAVE_GCC_BYTESWAP) From 3f968933f42506593be304641625c45a2b6f9a12 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 17 Nov 2015 10:49:30 +0900 Subject: [PATCH 48/69] Use std::wstring::empty() rather than size() == 0. Depending on the implementation, empty() can be more efficient than size(). --- taglib/toolkit/tstring.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/taglib/toolkit/tstring.cpp b/taglib/toolkit/tstring.cpp index 258e1fcf..e21316b2 100644 --- a/taglib/toolkit/tstring.cpp +++ b/taglib/toolkit/tstring.cpp @@ -386,7 +386,7 @@ TagLib::uint String::length() const bool String::isEmpty() const { - return d->data.size() == 0; + return d->data.empty(); } bool String::isNull() const From 3128f425b8e8ce7eea238708420f362ee7d5fb49 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 17 Nov 2015 11:06:19 +0900 Subject: [PATCH 49/69] vsnprintf()/vsprintf() does not necessarily return -1 when failed. --- taglib/toolkit/tutils.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/taglib/toolkit/tutils.h b/taglib/toolkit/tutils.h index 36e0afd3..0655342a 100644 --- a/taglib/toolkit/tutils.h +++ b/taglib/toolkit/tutils.h @@ -213,10 +213,10 @@ namespace TagLib va_end(args); - if(length != -1) + if(length > 0) return String(buf); else - return String::null; + return String(); } /*! From 1a942627bfd6f619262fa51944fcd429c507c6c5 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 17 Nov 2015 11:29:52 +0900 Subject: [PATCH 50/69] Add String::clear() method to clear the string. --- taglib/toolkit/tstring.cpp | 6 ++++++ taglib/toolkit/tstring.h | 5 +++++ tests/test_string.cpp | 4 ++++ 3 files changed, 15 insertions(+) diff --git a/taglib/toolkit/tstring.cpp b/taglib/toolkit/tstring.cpp index e21316b2..d623d237 100644 --- a/taglib/toolkit/tstring.cpp +++ b/taglib/toolkit/tstring.cpp @@ -358,6 +358,12 @@ String &String::append(const String &s) return *this; } +String & String::clear() +{ + *this = String(); + return *this; +} + String String::upper() const { String s; diff --git a/taglib/toolkit/tstring.h b/taglib/toolkit/tstring.h index 8b739882..68592f40 100644 --- a/taglib/toolkit/tstring.h +++ b/taglib/toolkit/tstring.h @@ -299,6 +299,11 @@ namespace TagLib { */ String &append(const String &s); + /*! + * Clears the string. + */ + String &clear(); + /*! * Returns an upper case version of the string. * diff --git a/tests/test_string.cpp b/tests/test_string.cpp index afb2aa23..24816af9 100644 --- a/tests/test_string.cpp +++ b/tests/test_string.cpp @@ -67,6 +67,10 @@ public: CPPUNIT_ASSERT(s != L"taglib"); CPPUNIT_ASSERT(s != L"taglib string taglib"); + s.clear(); + CPPUNIT_ASSERT(s.isEmpty()); + CPPUNIT_ASSERT(!s.isNull()); + String unicode("José Carlos", String::UTF8); CPPUNIT_ASSERT(strcmp(unicode.toCString(), "Jos\xe9 Carlos") == 0); From 6b7a2c4cf74fe8d73a6c247ba6b078f9a70e6ff2 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 17 Nov 2015 11:41:13 +0900 Subject: [PATCH 51/69] Add some notes about String::isNull() and String::null. --- taglib/toolkit/tstring.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/taglib/toolkit/tstring.h b/taglib/toolkit/tstring.h index 68592f40..a2ee19ff 100644 --- a/taglib/toolkit/tstring.h +++ b/taglib/toolkit/tstring.h @@ -332,7 +332,9 @@ namespace TagLib { * Returns true if this string is null -- i.e. it is a copy of the * String::null string. * - * \note A string can be empty and not null. + * \note A string can be empty and not null. So do not use this method to + * check if the string is empty. + * * \see isEmpty() */ bool isNull() const; @@ -508,6 +510,9 @@ namespace TagLib { /*! * A null string provided for convenience. + * + * \warning Do not modify this variable. It will mess up the internal state + * of TagLib. */ static String null; From 4bac99e3da324b736c538d586b0bcbc01b1d307b Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 17 Nov 2015 13:06:03 +0900 Subject: [PATCH 52/69] Add some notes about ByteVector::isNull() and ByteVector::null. --- taglib/toolkit/tbytevector.h | 7 ++++++- tests/test_bytevector.cpp | 4 ++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/taglib/toolkit/tbytevector.h b/taglib/toolkit/tbytevector.h index 793b414c..98b5d8b5 100644 --- a/taglib/toolkit/tbytevector.h +++ b/taglib/toolkit/tbytevector.h @@ -261,7 +261,9 @@ namespace TagLib { /*! * Returns true if the vector is null. * - * \note A vector may be empty without being null. + * \note A vector may be empty without being null. So do not use this + * method to check if the vector is empty. + * * \see isEmpty() */ bool isNull() const; @@ -565,6 +567,9 @@ namespace TagLib { /*! * A static, empty ByteVector which is convenient and fast (since returning * an empty or "null" value does not require instantiating a new ByteVector). + * + * \warning Do not modify this variable. It will mess up the internal state + * of TagLib. */ static ByteVector null; diff --git a/tests/test_bytevector.cpp b/tests/test_bytevector.cpp index 1c2ca904..4b0c098f 100644 --- a/tests/test_bytevector.cpp +++ b/tests/test_bytevector.cpp @@ -124,6 +124,10 @@ public: CPPUNIT_ASSERT(i.containsAt(j, 5, 0)); CPPUNIT_ASSERT(i.containsAt(j, 6, 1)); CPPUNIT_ASSERT(i.containsAt(j, 6, 1, 3)); + + i.clear(); + CPPUNIT_ASSERT(i.isEmpty()); + CPPUNIT_ASSERT(!i.isNull()); } void testFind1() From 76de4234a1928e9dd9af23320880d069817b5000 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 17 Nov 2015 15:05:43 +0900 Subject: [PATCH 53/69] Add a test for the CRC checksum of Ogg pages. --- tests/test_ogg.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_ogg.cpp b/tests/test_ogg.cpp index 210fca2f..90ae162d 100644 --- a/tests/test_ogg.cpp +++ b/tests/test_ogg.cpp @@ -21,6 +21,7 @@ class TestOGG : public CppUnit::TestFixture CPPUNIT_TEST(testDictInterface1); CPPUNIT_TEST(testDictInterface2); CPPUNIT_TEST(testAudioProperties); + CPPUNIT_TEST(testPageChecksum); CPPUNIT_TEST_SUITE_END(); public: @@ -134,6 +135,30 @@ public: CPPUNIT_ASSERT_EQUAL(112000, f.audioProperties()->bitrateNominal()); CPPUNIT_ASSERT_EQUAL(0, f.audioProperties()->bitrateMinimum()); } + + void testPageChecksum() + { + ScopedFileCopy copy("empty", ".ogg"); + + { + Vorbis::File f(copy.fileName().c_str()); + f.tag()->setArtist("The Artist"); + f.save(); + + f.seek(0x50); + CPPUNIT_ASSERT_EQUAL((TagLib::uint)0x3d3bd92d, f.readBlock(4).toUInt(0, true)); + } + { + Vorbis::File f(copy.fileName().c_str()); + f.tag()->setArtist("The Artist 2"); + f.save(); + + f.seek(0x50); + CPPUNIT_ASSERT_EQUAL((TagLib::uint)0xd985291c, f.readBlock(4).toUInt(0, true)); + } + + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestOGG); From 4f3844114abba69b8e03c9f914846a699619bec4 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 18 Nov 2015 10:29:25 +0900 Subject: [PATCH 54/69] Mark some variables and functions deprecated. ByteVector::null, ByteVector::isNull(), String::null, String::isNull() have got marked deprecated. It's dangerous to use them, and they will be removed in taglib2. --- taglib/toolkit/tbytevector.h | 6 ++++++ taglib/toolkit/tstring.h | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/taglib/toolkit/tbytevector.h b/taglib/toolkit/tbytevector.h index 98b5d8b5..00b833f5 100644 --- a/taglib/toolkit/tbytevector.h +++ b/taglib/toolkit/tbytevector.h @@ -265,7 +265,10 @@ namespace TagLib { * method to check if the vector is empty. * * \see isEmpty() + * + * \deprecated */ + // BIC: remove bool isNull() const; /*! @@ -570,7 +573,10 @@ namespace TagLib { * * \warning Do not modify this variable. It will mess up the internal state * of TagLib. + * + * \deprecated */ + // BIC: remove static ByteVector null; /*! diff --git a/taglib/toolkit/tstring.h b/taglib/toolkit/tstring.h index a2ee19ff..2263e5e4 100644 --- a/taglib/toolkit/tstring.h +++ b/taglib/toolkit/tstring.h @@ -336,7 +336,10 @@ namespace TagLib { * check if the string is empty. * * \see isEmpty() + * + * \deprecated */ + // BIC: remove bool isNull() const; /*! @@ -513,7 +516,10 @@ namespace TagLib { * * \warning Do not modify this variable. It will mess up the internal state * of TagLib. + * + * \deprecated */ + // BIC: remove static String null; protected: From 69921b80906172a031f7771ea90d501a535dd912 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 18 Nov 2015 14:17:29 +0900 Subject: [PATCH 55/69] Amend the comment on MP4::File::hasMP4Tag(). --- taglib/mp4/mp4file.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/taglib/mp4/mp4file.h b/taglib/mp4/mp4file.h index 8c049301..40fcde78 100644 --- a/taglib/mp4/mp4file.h +++ b/taglib/mp4/mp4file.h @@ -118,7 +118,8 @@ namespace TagLib { bool save(); /*! - * Returns whether or not the file on disk actually has an MP4 tag. + * Returns whether or not the file on disk actually has an MP4 tag, or the + * file has a Metadata Item List (ilst) atom. */ bool hasMP4Tag() const; From b01fecd280ca290b7cb41eba48dc25a112b555b7 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 18 Nov 2015 14:25:22 +0900 Subject: [PATCH 56/69] Separate some tests for MP4::File::hasMP4Tag(). --- tests/test_mp4.cpp | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/tests/test_mp4.cpp b/tests/test_mp4.cpp index ed93f043..c110a612 100644 --- a/tests/test_mp4.cpp +++ b/tests/test_mp4.cpp @@ -19,6 +19,7 @@ class TestMP4 : public CppUnit::TestFixture CPPUNIT_TEST(testPropertiesALAC); CPPUNIT_TEST(testFreeForm); CPPUNIT_TEST(testCheckValid); + CPPUNIT_TEST(testHasTag); CPPUNIT_TEST(testIsEmpty); CPPUNIT_TEST(testUpdateStco); CPPUNIT_TEST(testSaveExisingWhenIlstIsLast); @@ -67,12 +68,30 @@ public: { MP4::File f(TEST_FILE_PATH_C("empty.aiff")); CPPUNIT_ASSERT(!f.isValid()); - MP4::File f2(TEST_FILE_PATH_C("has-tags.m4a")); - CPPUNIT_ASSERT(f2.isValid()); - CPPUNIT_ASSERT(f2.hasMP4Tag()); - MP4::File f3(TEST_FILE_PATH_C("no-tags.m4a")); - CPPUNIT_ASSERT(f3.isValid()); - CPPUNIT_ASSERT(!f3.hasMP4Tag()); + } + + void testHasTag() + { + { + MP4::File f(TEST_FILE_PATH_C("has-tags.m4a")); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT(f.hasMP4Tag()); + } + + ScopedFileCopy copy("no-tags", ".m4a"); + + { + MP4::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT(!f.hasMP4Tag()); + f.tag()->setTitle("TITLE"); + f.save(); + } + { + MP4::File f(copy.fileName().c_str()); + CPPUNIT_ASSERT(f.isValid()); + CPPUNIT_ASSERT(f.hasMP4Tag()); + } } void testIsEmpty() From 0f00dfc778483c3ae8a01799947a24ccbab934ed Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 18 Nov 2015 15:14:50 +0900 Subject: [PATCH 57/69] Add a comment to List::isEmpty(). --- taglib/toolkit/tlist.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/taglib/toolkit/tlist.h b/taglib/toolkit/tlist.h index 4277a182..7375d752 100644 --- a/taglib/toolkit/tlist.h +++ b/taglib/toolkit/tlist.h @@ -146,8 +146,16 @@ namespace TagLib { /*! * Returns the number of elements in the list. + * + * \see isEmpty() */ uint size() const; + + /*! + * Returns whether or not the list is empty. + * + * \see size() + */ bool isEmpty() const; /*! From 07b26ab3e4a6f442d46b808b0053e8903830eae8 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 18 Nov 2015 15:29:04 +0900 Subject: [PATCH 58/69] Make use of List iterators and setAutoDelete(). --- taglib/mp4/mp4atom.cpp | 45 +++++++++++++++++++----------------------- taglib/mp4/mp4atom.h | 32 +++++++++++++++--------------- 2 files changed, 36 insertions(+), 41 deletions(-) diff --git a/taglib/mp4/mp4atom.cpp b/taglib/mp4/mp4atom.cpp index 5e94690f..681589d8 100644 --- a/taglib/mp4/mp4atom.cpp +++ b/taglib/mp4/mp4atom.cpp @@ -43,6 +43,8 @@ const char *MP4::Atom::containers[11] = { MP4::Atom::Atom(File *file) { + children.setAutoDelete(true); + offset = file->tell(); ByteVector header = file->readBlock(8); if (header.size() != 8) { @@ -106,10 +108,6 @@ MP4::Atom::Atom(File *file) MP4::Atom::~Atom() { - for(unsigned int i = 0; i < children.size(); i++) { - delete children[i]; - } - children.clear(); } MP4::Atom * @@ -118,9 +116,9 @@ MP4::Atom::find(const char *name1, const char *name2, const char *name3, const c if(name1 == 0) { return this; } - for(unsigned int i = 0; i < children.size(); i++) { - if(children[i]->name == name1) { - return children[i]->find(name2, name3, name4); + for(AtomList::ConstIterator it = children.begin(); it != children.end(); ++it) { + if((*it)->name == name1) { + return (*it)->find(name2, name3, name4); } } return 0; @@ -130,12 +128,12 @@ MP4::AtomList MP4::Atom::findall(const char *name, bool recursive) { MP4::AtomList result; - for(unsigned int i = 0; i < children.size(); i++) { - if(children[i]->name == name) { - result.append(children[i]); + for(AtomList::ConstIterator it = children.begin(); it != children.end(); ++it) { + if((*it)->name == name) { + result.append(*it); } if(recursive) { - result.append(children[i]->findall(name, recursive)); + result.append((*it)->findall(name, recursive)); } } return result; @@ -148,9 +146,9 @@ MP4::Atom::path(MP4::AtomList &path, const char *name1, const char *name2, const if(name1 == 0) { return true; } - for(unsigned int i = 0; i < children.size(); i++) { - if(children[i]->name == name1) { - return children[i]->path(path, name2, name3); + for(AtomList::ConstIterator it = children.begin(); it != children.end(); ++it) { + if((*it)->name == name1) { + return (*it)->path(path, name2, name3); } } return false; @@ -158,6 +156,8 @@ MP4::Atom::path(MP4::AtomList &path, const char *name1, const char *name2, const MP4::Atoms::Atoms(File *file) { + atoms.setAutoDelete(true); + file->seek(0, File::End); long end = file->tell(); file->seek(0); @@ -171,18 +171,14 @@ MP4::Atoms::Atoms(File *file) MP4::Atoms::~Atoms() { - for(unsigned int i = 0; i < atoms.size(); i++) { - delete atoms[i]; - } - atoms.clear(); } MP4::Atom * MP4::Atoms::find(const char *name1, const char *name2, const char *name3, const char *name4) { - for(unsigned int i = 0; i < atoms.size(); i++) { - if(atoms[i]->name == name1) { - return atoms[i]->find(name2, name3, name4); + for(AtomList::ConstIterator it = atoms.begin(); it != atoms.end(); ++it) { + if((*it)->name == name1) { + return (*it)->find(name2, name3, name4); } } return 0; @@ -192,9 +188,9 @@ MP4::AtomList MP4::Atoms::path(const char *name1, const char *name2, const char *name3, const char *name4) { MP4::AtomList path; - for(unsigned int i = 0; i < atoms.size(); i++) { - if(atoms[i]->name == name1) { - if(!atoms[i]->path(path, name2, name3, name4)) { + for(AtomList::ConstIterator it = atoms.begin(); it != atoms.end(); ++it) { + if((*it)->name == name1) { + if(!(*it)->path(path, name2, name3, name4)) { path.clear(); } return path; @@ -202,4 +198,3 @@ MP4::Atoms::path(const char *name1, const char *name2, const char *name3, const } return path; } - diff --git a/taglib/mp4/mp4atom.h b/taglib/mp4/mp4atom.h index 6cdb1d42..cbb0d10a 100644 --- a/taglib/mp4/mp4atom.h +++ b/taglib/mp4/mp4atom.h @@ -77,29 +77,29 @@ namespace TagLib { class Atom { public: - Atom(File *file); - ~Atom(); - Atom *find(const char *name1, const char *name2 = 0, const char *name3 = 0, const char *name4 = 0); - bool path(AtomList &path, const char *name1, const char *name2 = 0, const char *name3 = 0); - AtomList findall(const char *name, bool recursive = false); - long offset; - long length; - TagLib::ByteVector name; - AtomList children; + Atom(File *file); + ~Atom(); + Atom *find(const char *name1, const char *name2 = 0, const char *name3 = 0, const char *name4 = 0); + bool path(AtomList &path, const char *name1, const char *name2 = 0, const char *name3 = 0); + AtomList findall(const char *name, bool recursive = false); + long offset; + long length; + TagLib::ByteVector name; + AtomList children; private: - static const int numContainers = 11; - static const char *containers[11]; + static const int numContainers = 11; + static const char *containers[11]; }; //! Root-level atoms class Atoms { public: - Atoms(File *file); - ~Atoms(); - Atom *find(const char *name1, const char *name2 = 0, const char *name3 = 0, const char *name4 = 0); - AtomList path(const char *name1, const char *name2 = 0, const char *name3 = 0, const char *name4 = 0); - AtomList atoms; + Atoms(File *file); + ~Atoms(); + Atom *find(const char *name1, const char *name2 = 0, const char *name3 = 0, const char *name4 = 0); + AtomList path(const char *name1, const char *name2 = 0, const char *name3 = 0, const char *name4 = 0); + AtomList atoms; }; } From 288e97ad44e26e6697cda692f60f8f327c2ab469 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 18 Nov 2015 16:33:28 +0900 Subject: [PATCH 59/69] Make use of List iterators. --- taglib/mp4/mp4tag.cpp | 91 +++++++++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 37 deletions(-) diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index ce557c0f..a84f54e5 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -35,21 +35,23 @@ using namespace TagLib; class MP4::Tag::TagPrivate { public: - TagPrivate() : file(0), atoms(0) {} - ~TagPrivate() {} + TagPrivate() : + file(0), + atoms(0) {} + TagLib::File *file; Atoms *atoms; ItemMap items; }; -MP4::Tag::Tag() +MP4::Tag::Tag() : + d(new TagPrivate()) { - d = new TagPrivate; } -MP4::Tag::Tag(TagLib::File *file, MP4::Atoms *atoms) +MP4::Tag::Tag(TagLib::File *file, MP4::Atoms *atoms) : + d(new TagPrivate()) { - d = new TagPrivate; d->file = file; d->atoms = atoms; @@ -59,8 +61,8 @@ MP4::Tag::Tag(TagLib::File *file, MP4::Atoms *atoms) return; } - for(unsigned int i = 0; i < ilst->children.size(); i++) { - MP4::Atom *atom = ilst->children[i]; + for(AtomList::ConstIterator it = ilst->children.begin(); it != ilst->children.end(); ++it) { + MP4::Atom *atom = *it; file->seek(atom->offset + 8); if(atom->name == "----") { parseFreeForm(atom); @@ -149,8 +151,8 @@ MP4::Tag::parseData(const MP4::Atom *atom, int expectedFlags, bool freeForm) { AtomDataList data = parseData2(atom, expectedFlags, freeForm); ByteVectorList result; - for(uint i = 0; i < data.size(); i++) { - result.append(data[i].data); + for(AtomDataList::ConstIterator it = data.begin(); it != data.end(); ++it) { + result.append(it->data); } return result; } @@ -230,8 +232,8 @@ MP4::Tag::parseText(const MP4::Atom *atom, int expectedFlags) ByteVectorList data = parseData(atom, expectedFlags); if(data.size()) { StringList value; - for(unsigned int i = 0; i < data.size(); i++) { - value.append(String(data[i], String::UTF8)); + for(ByteVectorList::ConstIterator it = data.begin(); it != data.end(); ++it) { + value.append(String(*it, String::UTF8)); } addItem(atom->name, value); } @@ -242,18 +244,25 @@ MP4::Tag::parseFreeForm(const MP4::Atom *atom) { AtomDataList data = parseData2(atom, -1, true); if(data.size() > 2) { - String name = "----:" + String(data[0].data, String::UTF8) + ':' + String(data[1].data, String::UTF8); - AtomDataType type = data[2].type; - for(uint i = 2; i < data.size(); i++) { - if(data[i].type != type) { + AtomDataList::ConstIterator itBegin = data.begin(); + + String name = "----:"; + name += String((itBegin++)->data, String::UTF8); // data[0].data + name += ':'; + name += String((itBegin++)->data, String::UTF8); // data[1].data + + AtomDataType type = itBegin->type; // data[2].type + + for(AtomDataList::ConstIterator it = itBegin; it != data.end(); ++it) { + if(it->type != type) { debug("MP4: We currently don't support values with multiple types"); break; } } if(type == TypeUTF8) { StringList value; - for(uint i = 2; i < data.size(); i++) { - value.append(String(data[i].data, String::UTF8)); + for(AtomDataList::ConstIterator it = itBegin; it != data.end(); ++it) { + value.append(String(it->data, String::UTF8)); } Item item(value); item.setAtomDataType(type); @@ -261,8 +270,8 @@ MP4::Tag::parseFreeForm(const MP4::Atom *atom) } else { ByteVectorList value; - for(uint i = 2; i < data.size(); i++) { - value.append(data[i].data); + for(AtomDataList::ConstIterator it = itBegin; it != data.end(); ++it) { + value.append(it->data); } Item item(value); item.setAtomDataType(type); @@ -300,7 +309,7 @@ MP4::Tag::parseCovr(const MP4::Atom *atom) } pos += length; } - if(value.size() > 0) + if(!value.isEmpty()) addItem(atom->name, value); } @@ -406,9 +415,9 @@ MP4::Tag::renderCovr(const ByteVector &name, const MP4::Item &item) const { ByteVector data; MP4::CoverArtList value = item.toCoverArtList(); - for(unsigned int i = 0; i < value.size(); i++) { - data.append(renderAtom("data", ByteVector::fromUInt(value[i].format()) + - ByteVector(4, '\0') + value[i].data())); + for(MP4::CoverArtList::ConstIterator it = value.begin(); it != value.end(); ++it) { + data.append(renderAtom("data", ByteVector::fromUInt(it->format()) + + ByteVector(4, '\0') + it->data())); } return renderAtom(name, data); } @@ -505,20 +514,26 @@ MP4::Tag::save() void MP4::Tag::updateParents(const AtomList &path, long delta, int ignore) { - for(unsigned int i = 0; i < path.size() - ignore; i++) { - d->file->seek(path[i]->offset); + if(static_cast(path.size()) <= ignore) + return; + + AtomList::ConstIterator itEnd = path.end(); + std::advance(itEnd, 0 - ignore); + + for(AtomList::ConstIterator it = path.begin(); it != itEnd; ++it) { + d->file->seek((*it)->offset); long size = d->file->readBlock(4).toUInt(); // 64-bit if (size == 1) { d->file->seek(4, File::Current); // Skip name long long longSize = d->file->readBlock(8).toLongLong(); // Seek the offset of the 64-bit size - d->file->seek(path[i]->offset + 8); + d->file->seek((*it)->offset + 8); d->file->writeBlock(ByteVector::fromLongLong(longSize + delta)); } // 32-bit else { - d->file->seek(path[i]->offset); + d->file->seek((*it)->offset); d->file->writeBlock(ByteVector::fromUInt(size + delta)); } } @@ -530,8 +545,8 @@ MP4::Tag::updateOffsets(long delta, long offset) MP4::Atom *moov = d->atoms->find("moov"); if(moov) { MP4::AtomList stco = moov->findall("stco", true); - for(unsigned int i = 0; i < stco.size(); i++) { - MP4::Atom *atom = stco[i]; + for(MP4::AtomList::ConstIterator it = stco.begin(); it != stco.end(); ++it) { + MP4::Atom *atom = *it; if(atom->offset > offset) { atom->offset += delta; } @@ -551,8 +566,8 @@ MP4::Tag::updateOffsets(long delta, long offset) } MP4::AtomList co64 = moov->findall("co64", true); - for(unsigned int i = 0; i < co64.size(); i++) { - MP4::Atom *atom = co64[i]; + for(MP4::AtomList::ConstIterator it = co64.begin(); it != co64.end(); ++it) { + MP4::Atom *atom = *it; if(atom->offset > offset) { atom->offset += delta; } @@ -575,8 +590,8 @@ MP4::Tag::updateOffsets(long delta, long offset) MP4::Atom *moof = d->atoms->find("moof"); if(moof) { MP4::AtomList tfhd = moof->findall("tfhd", true); - for(unsigned int i = 0; i < tfhd.size(); i++) { - MP4::Atom *atom = tfhd[i]; + for(MP4::AtomList::ConstIterator it = tfhd.begin(); it != tfhd.end(); ++it) { + MP4::Atom *atom = *it; if(atom->offset > offset) { atom->offset += delta; } @@ -609,7 +624,7 @@ MP4::Tag::saveNew(ByteVector data) data = renderAtom("udta", data); } - long offset = path[path.size() - 1]->offset + 8; + long offset = path.back()->offset + 8; d->file->insert(data, offset, 0); updateParents(path, data.size()); @@ -619,11 +634,13 @@ MP4::Tag::saveNew(ByteVector data) void MP4::Tag::saveExisting(ByteVector data, const AtomList &path) { - MP4::Atom *ilst = path[path.size() - 1]; + AtomList::ConstIterator it = path.end(); + + MP4::Atom *ilst = *(--it); long offset = ilst->offset; long length = ilst->length; - MP4::Atom *meta = path[path.size() - 2]; + MP4::Atom *meta = *(--it); AtomList::ConstIterator index = meta->children.find(ilst); // check if there is an atom before 'ilst', and possibly use it as padding From 6e6e11f21a94511a32202cbbab45820f60f8ecc5 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 18 Nov 2015 16:57:41 +0900 Subject: [PATCH 60/69] Use List::isEmpty() rather than size() to check if the list is empty. std::list::empty() is guaranteed to be an O(1) operation. --- taglib/ape/apeitem.cpp | 4 ++-- taglib/ape/apetag.cpp | 2 +- taglib/mp4/mp4tag.cpp | 20 ++++++++++---------- taglib/ogg/xiphcomment.cpp | 2 +- taglib/xm/xmfile.cpp | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/taglib/ape/apeitem.cpp b/taglib/ape/apeitem.cpp index 49c3a665..442dce63 100644 --- a/taglib/ape/apeitem.cpp +++ b/taglib/ape/apeitem.cpp @@ -177,7 +177,7 @@ int APE::Item::size() const int result = 8 + d->key.size() /* d->key.data(String::UTF8).size() */ + 1; switch (d->type) { case Text: - if(d->text.size()) { + if(!d->text.isEmpty()) { StringList::ConstIterator it = d->text.begin(); result += it->data(String::UTF8).size(); @@ -249,7 +249,7 @@ void APE::Item::parse(const ByteVector &data) setReadOnly(flags & 1); setType(ItemTypes((flags >> 1) & 3)); - if(Text == d->type) + if(Text == d->type) d->text = StringList(ByteVectorList::split(value, '\0'), String::UTF8); else d->value = value; diff --git a/taglib/ape/apetag.cpp b/taglib/ape/apetag.cpp index 312533fe..7115fb47 100644 --- a/taglib/ape/apetag.cpp +++ b/taglib/ape/apetag.cpp @@ -247,7 +247,7 @@ PropertyMap APE::Tag::setProperties(const PropertyMap &origProps) if(!checkKey(tagName)) invalid.insert(it->first, it->second); else if(!(itemListMap().contains(tagName)) || !(itemListMap()[tagName].values() == it->second)) { - if(it->second.size() == 0) + if(it->second.isEmpty()) removeItem(tagName); else { StringList::ConstIterator valueIt = it->second.begin(); diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index a84f54e5..771fc59a 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -161,7 +161,7 @@ void MP4::Tag::parseInt(const MP4::Atom *atom) { ByteVectorList data = parseData(atom); - if(data.size()) { + if(!data.isEmpty()) { addItem(atom->name, (int)data[0].toShort()); } } @@ -170,7 +170,7 @@ void MP4::Tag::parseUInt(const MP4::Atom *atom) { ByteVectorList data = parseData(atom); - if(data.size()) { + if(!data.isEmpty()) { addItem(atom->name, data[0].toUInt()); } } @@ -179,7 +179,7 @@ void MP4::Tag::parseLongLong(const MP4::Atom *atom) { ByteVectorList data = parseData(atom); - if(data.size()) { + if(!data.isEmpty()) { addItem(atom->name, data[0].toLongLong()); } } @@ -188,7 +188,7 @@ void MP4::Tag::parseByte(const MP4::Atom *atom) { ByteVectorList data = parseData(atom); - if(data.size()) { + if(!data.isEmpty()) { addItem(atom->name, (uchar)data[0].at(0)); } } @@ -197,7 +197,7 @@ void MP4::Tag::parseGnre(const MP4::Atom *atom) { ByteVectorList data = parseData(atom); - if(data.size()) { + if(!data.isEmpty()) { int idx = (int)data[0].toShort(); if(idx > 0) { addItem("\251gen", StringList(ID3v1::genre(idx - 1))); @@ -209,7 +209,7 @@ void MP4::Tag::parseIntPair(const MP4::Atom *atom) { ByteVectorList data = parseData(atom); - if(data.size()) { + if(!data.isEmpty()) { const int a = data[0].toShort(2U); const int b = data[0].toShort(4U); addItem(atom->name, MP4::Item(a, b)); @@ -220,7 +220,7 @@ void MP4::Tag::parseBool(const MP4::Atom *atom) { ByteVectorList data = parseData(atom); - if(data.size()) { + if(!data.isEmpty()) { bool value = data[0].size() ? data[0][0] != '\0' : false; addItem(atom->name, value); } @@ -230,7 +230,7 @@ void MP4::Tag::parseText(const MP4::Atom *atom, int expectedFlags) { ByteVectorList data = parseData(atom, expectedFlags); - if(data.size()) { + if(!data.isEmpty()) { StringList value; for(ByteVectorList::ConstIterator it = data.begin(); it != data.end(); ++it) { value.append(String(*it, String::UTF8)); @@ -426,7 +426,7 @@ ByteVector MP4::Tag::renderFreeForm(const String &name, const MP4::Item &item) const { StringList header = StringList::split(name, ":"); - if (header.size() != 3) { + if(header.size() != 3) { debug("MP4: Invalid free-form item name \"" + name + "\""); return ByteVector::null; } @@ -937,7 +937,7 @@ PropertyMap MP4::Tag::setProperties(const PropertyMap &props) if((it->first == "TRACKNUMBER" || it->first == "DISCNUMBER") && !it->second.isEmpty()) { int first = 0, second = 0; StringList parts = StringList::split(it->second.front(), "/"); - if(parts.size() > 0) { + if(!parts.isEmpty()) { first = parts[0].toInt(); if(parts.size() > 1) { second = parts[1].toInt(); diff --git a/taglib/ogg/xiphcomment.cpp b/taglib/ogg/xiphcomment.cpp index 9462607f..1da0edac 100644 --- a/taglib/ogg/xiphcomment.cpp +++ b/taglib/ogg/xiphcomment.cpp @@ -214,7 +214,7 @@ PropertyMap Ogg::XiphComment::setProperties(const PropertyMap &properties) invalid.insert(it->first, it->second); else if(!d->fieldListMap.contains(it->first) || !(it->second == d->fieldListMap[it->first])) { const StringList &sl = it->second; - if(sl.size() == 0) + if(sl.isEmpty()) // zero size string list -> remove the tag with all values removeField(it->first); else { diff --git a/taglib/xm/xmfile.cpp b/taglib/xm/xmfile.cpp index 7fe6f108..3625d213 100644 --- a/taglib/xm/xmfile.cpp +++ b/taglib/xm/xmfile.cpp @@ -635,7 +635,7 @@ void XM::File::read(bool) d->properties.setSampleCount(sumSampleCount); String comment(intrumentNames.toString("\n")); - if(sampleNames.size() > 0) { + if(!sampleNames.isEmpty()) { comment += "\n"; comment += sampleNames.toString("\n"); } From 47860c20f4793e64afeaf1a88d469613799bfe40 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 18 Nov 2015 17:06:49 +0900 Subject: [PATCH 61/69] Make use of List iterators. --- taglib/mp4/mp4tag.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index 771fc59a..52f66906 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -332,8 +332,8 @@ ByteVector MP4::Tag::renderData(const ByteVector &name, int flags, const ByteVectorList &data) const { ByteVector result; - for(unsigned int i = 0; i < data.size(); i++) { - result.append(renderAtom("data", ByteVector::fromUInt(flags) + ByteVector(4, '\0') + data[i])); + for(ByteVectorList::ConstIterator it = data.begin(); it != data.end(); ++it) { + result.append(renderAtom("data", ByteVector::fromUInt(flags) + ByteVector(4, '\0') + *it)); } return renderAtom(name, result); } @@ -404,8 +404,8 @@ MP4::Tag::renderText(const ByteVector &name, const MP4::Item &item, int flags) c { ByteVectorList data; StringList value = item.toStringList(); - for(unsigned int i = 0; i < value.size(); i++) { - data.append(value[i].data(String::UTF8)); + for(StringList::ConstIterator it = value.begin(); it != value.end(); ++it) { + data.append(it->data(String::UTF8)); } return renderData(name, flags, data); } @@ -444,14 +444,14 @@ MP4::Tag::renderFreeForm(const String &name, const MP4::Item &item) const } if(type == TypeUTF8) { StringList value = item.toStringList(); - for(unsigned int i = 0; i < value.size(); i++) { - data.append(renderAtom("data", ByteVector::fromUInt(type) + ByteVector(4, '\0') + value[i].data(String::UTF8))); + for(StringList::ConstIterator it = value.begin(); it != value.end(); ++it) { + data.append(renderAtom("data", ByteVector::fromUInt(type) + ByteVector(4, '\0') + it->data(String::UTF8))); } } else { ByteVectorList value = item.toByteVectorList(); - for(unsigned int i = 0; i < value.size(); i++) { - data.append(renderAtom("data", ByteVector::fromUInt(type) + ByteVector(4, '\0') + value[i])); + for(ByteVectorList::ConstIterator it = value.begin(); it != value.end(); ++it) { + data.append(renderAtom("data", ByteVector::fromUInt(type) + ByteVector(4, '\0') + *it)); } } return renderAtom("----", data); @@ -875,8 +875,7 @@ PropertyMap MP4::Tag::properties() const } PropertyMap props; - MP4::ItemMap::ConstIterator it = d->items.begin(); - for(; it != d->items.end(); ++it) { + for(MP4::ItemMap::ConstIterator it = d->items.begin(); it != d->items.end(); ++it) { if(keyMap.contains(it->first)) { String key = keyMap[it->first]; if(key == "TRACKNUMBER" || key == "DISCNUMBER") { @@ -906,8 +905,7 @@ PropertyMap MP4::Tag::properties() const void MP4::Tag::removeUnsupportedProperties(const StringList &props) { - StringList::ConstIterator it = props.begin(); - for(; it != props.end(); ++it) + for(StringList::ConstIterator it = props.begin(); it != props.end(); ++it) d->items.erase(*it); } @@ -922,16 +920,14 @@ PropertyMap MP4::Tag::setProperties(const PropertyMap &props) } PropertyMap origProps = properties(); - PropertyMap::ConstIterator it = origProps.begin(); - for(; it != origProps.end(); ++it) { + for(PropertyMap::ConstIterator it = origProps.begin(); it != origProps.end(); ++it) { if(!props.contains(it->first) || props[it->first].isEmpty()) { d->items.erase(reverseKeyMap[it->first]); } } PropertyMap ignoredProps; - it = props.begin(); - for(; it != props.end(); ++it) { + for(PropertyMap::ConstIterator it = props.begin(); it != props.end(); ++it) { if(reverseKeyMap.contains(it->first)) { String name = reverseKeyMap[it->first]; if((it->first == "TRACKNUMBER" || it->first == "DISCNUMBER") && !it->second.isEmpty()) { From c6443dabc6b6511690a07ec8654c6f35306256cc Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 18 Nov 2015 17:58:13 +0900 Subject: [PATCH 62/69] Use the same type name between a List and its iterator. --- taglib/ape/apetag.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/taglib/ape/apetag.cpp b/taglib/ape/apetag.cpp index 7be798b0..c633d575 100644 --- a/taglib/ape/apetag.cpp +++ b/taglib/ape/apetag.cpp @@ -351,13 +351,9 @@ ByteVector APE::Tag::render() const ByteVector data; uint itemCount = 0; - { - for(Map::ConstIterator it = d->itemListMap.begin(); - it != d->itemListMap.end(); ++it) - { - data.append(it->second.render()); - itemCount++; - } + for(ItemListMap::ConstIterator it = d->itemListMap.begin(); it != d->itemListMap.end(); ++it) { + data.append(it->second.render()); + itemCount++; } d->footer.setItemCount(itemCount); From 7e85d9b2020194427b509b0b73aaa5fc4c77dfd6 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Tue, 6 Jan 2015 11:32:31 +0900 Subject: [PATCH 63/69] Simplify overly complicated ByteVector::mid() implementation. Especially remove the useless nested RefCounters. --- taglib/toolkit/tbytevector.cpp | 246 ++++++++++++--------------------- taglib/toolkit/tbytevector.h | 10 ++ tests/test_bytevector.cpp | 19 +++ 3 files changed, 119 insertions(+), 156 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 946c1c14..1e47ee2e 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -49,7 +49,7 @@ // // http://www.informit.com/isapi/product_id~{9C84DAB4-FE6E-49C5-BB0A-FB50331233EA}/content/index.asp -#define DATA(x) (&(x->data->data[0])) +#define DATA(x) (&(*(x)->data)[0]) namespace TagLib { @@ -275,6 +275,8 @@ ByteVector fromFloat(TFloat value) template long double toFloat80(const ByteVector &v, size_t offset) { + using std::swap; + if(offset > v.size() - 10) { debug("toFloat80() - offset is out of range. Returning 0."); return 0.0; @@ -284,11 +286,11 @@ long double toFloat80(const ByteVector &v, size_t offset) ::memcpy(bytes, v.data() + offset, 10); if(ENDIAN == Utils::LittleEndian) { - std::swap(bytes[0], bytes[9]); - std::swap(bytes[1], bytes[8]); - std::swap(bytes[2], bytes[7]); - std::swap(bytes[3], bytes[6]); - std::swap(bytes[4], bytes[5]); + swap(bytes[0], bytes[9]); + swap(bytes[1], bytes[8]); + swap(bytes[2], bytes[7]); + swap(bytes[3], bytes[6]); + swap(bytes[4], bytes[5]); } // 1-bit sign @@ -326,108 +328,42 @@ long double toFloat80(const ByteVector &v, size_t offset) return val; } -class DataPrivate : public RefCounter +class ByteVector::ByteVectorPrivate { public: - DataPrivate() - { - } + ByteVectorPrivate(uint l, char c) : + counter(new RefCounter()), + data(new std::vector(l, c)), + offset(0), + length(l) {} - DataPrivate(const std::vector &v, uint offset, uint length) - : data(v.begin() + offset, v.begin() + offset + length) - { - } + ByteVectorPrivate(const char *s, uint l) : + counter(new RefCounter()), + data(new std::vector(s, s + l)), + offset(0), + length(l) {} - // A char* can be an iterator. - DataPrivate(const char *begin, const char *end) - : data(begin, end) + ByteVectorPrivate(const ByteVectorPrivate &d, uint o, uint l) : + counter(d.counter), + data(d.data), + offset(d.offset + o), + length(l) { - } - - DataPrivate(uint len, char c) - : data(len, c) - { - } - - std::vector data; -}; - -class ByteVector::ByteVectorPrivate : public RefCounter -{ -public: - ByteVectorPrivate() - : RefCounter() - , data(new DataPrivate()) - , offset(0) - , length(0) - { - } - - ByteVectorPrivate(ByteVectorPrivate *d, uint o, uint l) - : RefCounter() - , data(d->data) - , offset(d->offset + o) - , length(l) - { - data->ref(); - } - - ByteVectorPrivate(const std::vector &v, uint o, uint l) - : RefCounter() - , data(new DataPrivate(v, o, l)) - , offset(0) - , length(l) - { - } - - ByteVectorPrivate(uint l, char c) - : RefCounter() - , data(new DataPrivate(l, c)) - , offset(0) - , length(l) - { - } - - ByteVectorPrivate(const char *s, uint l) - : RefCounter() - , data(new DataPrivate(s, s + l)) - , offset(0) - , length(l) - { - } - - void detach() - { - if(data->count() > 1) { - data->deref(); - data = new DataPrivate(data->data, offset, length); - offset = 0; - } + counter->ref(); } ~ByteVectorPrivate() { - if(data->deref()) + if(counter->deref()) { + delete counter; delete data; - } - - ByteVectorPrivate &operator=(const ByteVectorPrivate &x) - { - if(&x != this) - { - if(data->deref()) - delete data; - - data = x.data; - data->ref(); } - - return *this; } - DataPrivate *data; - uint offset; - uint length; + RefCounter *counter; + std::vector *data; + uint offset; + uint length; }; //////////////////////////////////////////////////////////////////////////////// @@ -483,69 +419,67 @@ ByteVector ByteVector::fromFloat64BE(double value) // public members //////////////////////////////////////////////////////////////////////////////// -ByteVector::ByteVector() - : d(new ByteVectorPrivate()) +ByteVector::ByteVector() : + d(new ByteVectorPrivate(0, '\0')) { } -ByteVector::ByteVector(uint size, char value) - : d(new ByteVectorPrivate(size, value)) +ByteVector::ByteVector(uint size, char value) : + d(new ByteVectorPrivate(size, value)) { } -ByteVector::ByteVector(const ByteVector &v) - : d(v.d) -{ - d->ref(); -} - -ByteVector::ByteVector(const ByteVector &v, uint offset, uint length) - : d(new ByteVectorPrivate(v.d, offset, length)) +ByteVector::ByteVector(const ByteVector &v) : + d(new ByteVectorPrivate(*v.d, 0, v.d->length)) { } -ByteVector::ByteVector(char c) - : d(new ByteVectorPrivate(1, c)) +ByteVector::ByteVector(const ByteVector &v, uint offset, uint length) : + d(new ByteVectorPrivate(*v.d, offset, length)) { } -ByteVector::ByteVector(const char *data, uint length) - : d(new ByteVectorPrivate(data, length)) +ByteVector::ByteVector(char c) : + d(new ByteVectorPrivate(1, c)) { } -ByteVector::ByteVector(const char *data) - : d(new ByteVectorPrivate(data, ::strlen(data))) +ByteVector::ByteVector(const char *data, uint length) : + d(new ByteVectorPrivate(data, length)) +{ +} + +ByteVector::ByteVector(const char *data) : + d(new ByteVectorPrivate(data, ::strlen(data))) { } ByteVector::~ByteVector() { - if(d->deref()) - delete d; + delete d; } ByteVector &ByteVector::setData(const char *s, uint length) { - *this = ByteVector(s, length); + ByteVector(s, length).swap(*this); return *this; } ByteVector &ByteVector::setData(const char *data) { - *this = ByteVector(data); + ByteVector(data).swap(*this); return *this; } char *ByteVector::data() { detach(); - return size() > 0 ? (DATA(d) + d->offset) : 0; + return (size() > 0) ? (DATA(d) + d->offset) : 0; } const char *ByteVector::data() const { - return size() > 0 ? (DATA(d) + d->offset) : 0; + return (size() > 0) ? (DATA(d) + d->offset) : 0; } ByteVector ByteVector::mid(uint index, uint length) const @@ -558,7 +492,7 @@ ByteVector ByteVector::mid(uint index, uint length) const char ByteVector::at(uint index) const { - return index < size() ? DATA(d)[d->offset + index] : 0; + return (index < size()) ? DATA(d)[d->offset + index] : 0; } int ByteVector::find(const ByteVector &pattern, uint offset, int byteAlign) const @@ -675,10 +609,8 @@ int ByteVector::endsWithPartialMatch(const ByteVector &pattern) const ByteVector &ByteVector::append(const ByteVector &v) { - if(v.d->length != 0) - { + if(v.d->length != 0) { detach(); - uint originalSize = size(); resize(originalSize + v.size()); ::memcpy(data() + originalSize, v.data(), v.size()); @@ -687,9 +619,15 @@ ByteVector &ByteVector::append(const ByteVector &v) return *this; } +ByteVector &ByteVector::append(char c) +{ + resize(size() + 1, c); + return *this; +} + ByteVector &ByteVector::clear() { - *this = ByteVector(); + ByteVector().swap(*this); return *this; } @@ -707,8 +645,8 @@ ByteVector &ByteVector::resize(uint size, char padding) // This doesn't reallocate the buffer, since std::vector::resize() doesn't // reallocate the buffer when shrinking. - d->data->data.resize(d->offset + d->length); - d->data->data.resize(d->offset + size, padding); + d->data->resize(d->offset + d->length); + d->data->resize(d->offset + size, padding); d->length = size; } @@ -719,29 +657,29 @@ ByteVector &ByteVector::resize(uint size, char padding) ByteVector::Iterator ByteVector::begin() { detach(); - return d->data->data.begin() + d->offset; + return d->data->begin() + d->offset; } ByteVector::ConstIterator ByteVector::begin() const { - return d->data->data.begin() + d->offset; + return d->data->begin() + d->offset; } ByteVector::Iterator ByteVector::end() { detach(); - return d->data->data.begin() + d->offset + d->length; + return d->data->begin() + d->offset + d->length; } ByteVector::ConstIterator ByteVector::end() const { - return d->data->data.begin() + d->offset + d->length; + return d->data->begin() + d->offset + d->length; } ByteVector::ReverseIterator ByteVector::rbegin() { detach(); - return d->data->data.rbegin() + (d->data->data.size() - (d->offset + d->length)); + return d->data->rbegin() + (d->data->size() - (d->offset + d->length)); } ByteVector::ConstReverseIterator ByteVector::rbegin() const @@ -754,7 +692,7 @@ ByteVector::ConstReverseIterator ByteVector::rbegin() const ByteVector::ReverseIterator ByteVector::rend() { detach(); - return d->data->data.rbegin() + (d->data->data.size() - d->offset); + return d->data->rbegin() + (d->data->size() - d->offset); } ByteVector::ConstReverseIterator ByteVector::rend() const @@ -859,13 +797,13 @@ long double ByteVector::toFloat80BE(size_t offset) const const char &ByteVector::operator[](int index) const { - return d->data->data[d->offset + index]; + return DATA(d)[d->offset + index]; } char &ByteVector::operator[](int index) { detach(); - return d->data->data[d->offset + index]; + return DATA(d)[d->offset + index]; } bool ByteVector::operator==(const ByteVector &v) const @@ -878,7 +816,7 @@ bool ByteVector::operator==(const ByteVector &v) const bool ByteVector::operator!=(const ByteVector &v) const { - return !operator==(v); + return !(*this == v); } bool ByteVector::operator==(const char *s) const @@ -891,7 +829,7 @@ bool ByteVector::operator==(const char *s) const bool ByteVector::operator!=(const char *s) const { - return !operator==(s); + return !(*this == s); } bool ByteVector::operator<(const ByteVector &v) const @@ -905,7 +843,7 @@ bool ByteVector::operator<(const ByteVector &v) const bool ByteVector::operator>(const ByteVector &v) const { - return v < *this; + return (v < *this); } ByteVector ByteVector::operator+(const ByteVector &v) const @@ -917,29 +855,29 @@ ByteVector ByteVector::operator+(const ByteVector &v) const ByteVector &ByteVector::operator=(const ByteVector &v) { - if(&v == this) - return *this; - - if(d->deref()) - delete d; - - d = v.d; - d->ref(); + ByteVector(v).swap(*this); return *this; } ByteVector &ByteVector::operator=(char c) { - *this = ByteVector(c); + ByteVector(c).swap(*this); return *this; } ByteVector &ByteVector::operator=(const char *data) { - *this = ByteVector(data); + ByteVector(data).swap(*this); return *this; } +void ByteVector::swap(ByteVector &v) +{ + using std::swap; + + swap(d, v.d); +} + ByteVector ByteVector::toHex() const { ByteVector encoded(size() * 2); @@ -960,15 +898,11 @@ ByteVector ByteVector::toHex() const void ByteVector::detach() { - if(d->data->count() > 1) { - d->data->deref(); - d->data = new DataPrivate(d->data->data, d->offset, d->length); - d->offset = 0; - } - - if(d->count() > 1) { - d->deref(); - d = new ByteVectorPrivate(d->data->data, d->offset, d->length); + if(d->counter->count() > 1) { + if(!isEmpty()) + ByteVector(DATA(d) + d->offset, d->length).swap(*this); + else + ByteVector().swap(*this); } } } diff --git a/taglib/toolkit/tbytevector.h b/taglib/toolkit/tbytevector.h index 00b833f5..e04d338d 100644 --- a/taglib/toolkit/tbytevector.h +++ b/taglib/toolkit/tbytevector.h @@ -201,6 +201,11 @@ namespace TagLib { */ ByteVector &append(const ByteVector &v); + /*! + * Appends \a c to the end of the ByteVector. + */ + ByteVector &append(char c); + /*! * Clears the data. */ @@ -567,6 +572,11 @@ namespace TagLib { */ ByteVector &operator=(const char *data); + /*! + * Exchanges the content of the ByteVector by the content of \a v. + */ + void swap(ByteVector &v); + /*! * A static, empty ByteVector which is convenient and fast (since returning * an empty or "null" value does not require instantiating a new ByteVector). diff --git a/tests/test_bytevector.cpp b/tests/test_bytevector.cpp index 4b0c098f..0ab68921 100644 --- a/tests/test_bytevector.cpp +++ b/tests/test_bytevector.cpp @@ -43,6 +43,7 @@ class TestByteVector : public CppUnit::TestFixture CPPUNIT_TEST(testReplace); CPPUNIT_TEST(testIterator); CPPUNIT_TEST(testResize); + CPPUNIT_TEST(testAppend); CPPUNIT_TEST_SUITE_END(); public: @@ -312,6 +313,8 @@ public: *it2 = 'I'; CPPUNIT_ASSERT_EQUAL('i', *it1); CPPUNIT_ASSERT_EQUAL('I', *it2); + CPPUNIT_ASSERT_EQUAL(ByteVector("taglib"), v1); + CPPUNIT_ASSERT_EQUAL(ByteVector("taglIb"), v2); ByteVector::ReverseIterator it3 = v1.rbegin(); ByteVector::ReverseIterator it4 = v2.rbegin(); @@ -324,6 +327,8 @@ public: *it4 = 'A'; CPPUNIT_ASSERT_EQUAL('a', *it3); CPPUNIT_ASSERT_EQUAL('A', *it4); + CPPUNIT_ASSERT_EQUAL(ByteVector("taglib"), v1); + CPPUNIT_ASSERT_EQUAL(ByteVector("tAglIb"), v2); ByteVector v3; v3 = ByteVector("0123456789").mid(3, 4); @@ -383,6 +388,20 @@ public: CPPUNIT_ASSERT_EQUAL(-1, c.find('C')); } + void testAppend() + { + ByteVector v1("taglib"); + ByteVector v2 = v1; + + v1.append("ABC"); + CPPUNIT_ASSERT_EQUAL(ByteVector("taglibABC"), v1); + v1.append('1'); + v1.append('2'); + v1.append('3'); + CPPUNIT_ASSERT_EQUAL(ByteVector("taglibABC123"), v1); + CPPUNIT_ASSERT_EQUAL(ByteVector("taglib"), v2); + } + }; CPPUNIT_TEST_SUITE_REGISTRATION(TestByteVector); From 0ffd2e8ab96051d6db3bd8cc6da46fab5754eb20 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 18 May 2015 01:12:21 +0900 Subject: [PATCH 64/69] Replace DATA macro with more straightforward notations. --- taglib/toolkit/tbytevector.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 1e47ee2e..70939dda 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -49,8 +49,6 @@ // // http://www.informit.com/isapi/product_id~{9C84DAB4-FE6E-49C5-BB0A-FB50331233EA}/content/index.asp -#define DATA(x) (&(*(x)->data)[0]) - namespace TagLib { static const char hexTable[17] = "0123456789abcdef"; @@ -474,12 +472,12 @@ ByteVector &ByteVector::setData(const char *data) char *ByteVector::data() { detach(); - return (size() > 0) ? (DATA(d) + d->offset) : 0; + return (size() > 0) ? (&d->data->front() + d->offset) : 0; } const char *ByteVector::data() const { - return (size() > 0) ? (DATA(d) + d->offset) : 0; + return (size() > 0) ? (&d->data->front() + d->offset) : 0; } ByteVector ByteVector::mid(uint index, uint length) const @@ -492,7 +490,7 @@ ByteVector ByteVector::mid(uint index, uint length) const char ByteVector::at(uint index) const { - return (index < size()) ? DATA(d)[d->offset + index] : 0; + return (index < size()) ? (*d->data)[d->offset + index] : 0; } int ByteVector::find(const ByteVector &pattern, uint offset, int byteAlign) const @@ -797,13 +795,13 @@ long double ByteVector::toFloat80BE(size_t offset) const const char &ByteVector::operator[](int index) const { - return DATA(d)[d->offset + index]; + return (*d->data)[d->offset + index]; } char &ByteVector::operator[](int index) { detach(); - return DATA(d)[d->offset + index]; + return (*d->data)[d->offset + index]; } bool ByteVector::operator==(const ByteVector &v) const @@ -900,7 +898,7 @@ void ByteVector::detach() { if(d->counter->count() > 1) { if(!isEmpty()) - ByteVector(DATA(d) + d->offset, d->length).swap(*this); + ByteVector(&d->data->front() + d->offset, d->length).swap(*this); else ByteVector().swap(*this); } From 3bce77a3591f012d924d001a9fe7d263e797f1a6 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Mon, 22 Jun 2015 10:37:20 +0900 Subject: [PATCH 65/69] Use linear search instead of the Knuth-Morris-Pratt algorithm in ByteVector::find(). In almost all cases, it handles too small data for KMP to work effectively. --- taglib/toolkit/tbytevector.cpp | 53 ++++++++++------------------------ 1 file changed, 16 insertions(+), 37 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 70939dda..636a1a34 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -99,10 +99,6 @@ static const uint crcTable[256] = { 0xbcb4666d, 0xb8757bda, 0xb5365d03, 0xb1f740b4 }; -/*! - * A templatized straightforward find that works with the types - * std::vector::iterator and std::vector::reverse_iterator. - */ template int findChar( const TIterator dataBegin, const TIterator dataEnd, @@ -125,10 +121,6 @@ int findChar( return -1; } -/*! - * A templatized KMP find that works with the types - * std::vector::iterator and std::vector::reverse_iterator. - */ template int findVector( const TIterator dataBegin, const TIterator dataEnd, @@ -140,46 +132,33 @@ int findVector( if(patternSize == 0 || offset + patternSize > dataSize) return -1; - // n % 0 is invalid - - if(byteAlign == 0) - return -1; - // Special case that pattern contains just single char. if(patternSize == 1) return findChar(dataBegin, dataEnd, *patternBegin, offset, byteAlign); - size_t lastOccurrence[256]; + // n % 0 is invalid - for(size_t i = 0; i < 256; ++i) - lastOccurrence[i] = patternSize; + if(byteAlign == 0) + return -1; - for(size_t i = 0; i < patternSize - 1; ++i) - lastOccurrence[static_cast(*(patternBegin + i))] = patternSize - i - 1; + // We don't use sophisticated algorithms like Knuth-Morris-Pratt here. - TIterator it = dataBegin + patternSize - 1 + offset; - while(true) { - TIterator itBuffer = it; - TIterator itPattern = patternBegin + patternSize - 1; + // In the current implementation of TagLib, data and patterns are too small + // for such algorithms to work effectively. - while(*itBuffer == *itPattern) { - if(itPattern == patternBegin) { - if((itBuffer - dataBegin - offset) % byteAlign == 0) - return (itBuffer - dataBegin); - else - break; - } + for(TIterator it = dataBegin + offset; it < dataEnd - patternSize + 1; it += byteAlign) { - --itBuffer; - --itPattern; + TIterator itData = it; + TIterator itPattern = patternBegin; + + while(*itData == *itPattern) { + ++itData; + ++itPattern; + + if(itPattern == patternEnd) + return (it - dataBegin); } - - const size_t step = lastOccurrence[static_cast(*it)]; - if(dataEnd - step <= it) - break; - - it += step; } return -1; From 10e1fcd686e4aebc008d1046b65d603a9af3353c Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Wed, 26 Aug 2015 16:38:06 +0900 Subject: [PATCH 66/69] Consistent notations between ByteVector::data() and at(). --- taglib/toolkit/tbytevector.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index 636a1a34..d2464974 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -451,12 +451,12 @@ ByteVector &ByteVector::setData(const char *data) char *ByteVector::data() { detach(); - return (size() > 0) ? (&d->data->front() + d->offset) : 0; + return (size() > 0) ? (&(*d->data)[d->offset]) : 0; } const char *ByteVector::data() const { - return (size() > 0) ? (&d->data->front() + d->offset) : 0; + return (size() > 0) ? (&(*d->data)[d->offset]) : 0; } ByteVector ByteVector::mid(uint index, uint length) const From e0f1151c5cdb4cc7b9125a70b019997b6e089382 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 19 Nov 2015 09:27:15 +0900 Subject: [PATCH 67/69] Resolve some conflicts before merging. --- taglib/toolkit/tbytevector.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index d2464974..c65962da 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -661,8 +661,9 @@ ByteVector::ReverseIterator ByteVector::rbegin() ByteVector::ConstReverseIterator ByteVector::rbegin() const { - // we need a const reference to the data vector so we can ensure the const version of rbegin() is called - const std::vector &v = d->data->data; + // Workaround for the Solaris Studio 12.4 compiler. + // We need a const reference to the data vector so we can ensure the const version of rbegin() is called. + const std::vector &v = *d->data; return v.rbegin() + (v.size() - (d->offset + d->length)); } @@ -674,8 +675,9 @@ ByteVector::ReverseIterator ByteVector::rend() ByteVector::ConstReverseIterator ByteVector::rend() const { - // we need a const reference to the data vector so we can ensure the const version of rbegin() is called - const std::vector &v = d->data->data; + // Workaround for the Solaris Studio 12.4 compiler. + // We need a const reference to the data vector so we can ensure the const version of rbegin() is called. + const std::vector &v = *d->data; return v.rbegin() + (v.size() - d->offset); } From 21788f4a2667695a2b57cb7463fa43f6b4539afb Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 19 Nov 2015 10:07:10 +0900 Subject: [PATCH 68/69] Efficient lookup for the ID3v2 frame ID table. Linear lookup is much faster and memory efficient when an array is very small. --- .../id3v2/frames/textidentificationframe.cpp | 2 +- taglib/mpeg/id3v2/frames/urllinkframe.cpp | 2 +- taglib/mpeg/id3v2/id3v2frame.cpp | 106 +++++++----------- taglib/mpeg/id3v2/id3v2frame.h | 4 +- 4 files changed, 44 insertions(+), 70 deletions(-) diff --git a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp index b77dd547..07bf0ede 100644 --- a/taglib/mpeg/id3v2/frames/textidentificationframe.cpp +++ b/taglib/mpeg/id3v2/frames/textidentificationframe.cpp @@ -146,7 +146,7 @@ PropertyMap TextIdentificationFrame::asProperties() const return makeTMCLProperties(); PropertyMap map; String tagName = frameIDToKey(frameID()); - if(tagName.isNull()) { + if(tagName.isEmpty()) { map.unsupportedData().append(frameID()); return map; } diff --git a/taglib/mpeg/id3v2/frames/urllinkframe.cpp b/taglib/mpeg/id3v2/frames/urllinkframe.cpp index 1225b524..e42d64d1 100644 --- a/taglib/mpeg/id3v2/frames/urllinkframe.cpp +++ b/taglib/mpeg/id3v2/frames/urllinkframe.cpp @@ -84,7 +84,7 @@ PropertyMap UrlLinkFrame::asProperties() const { String key = frameIDToKey(frameID()); PropertyMap map; - if(key.isNull()) + if(key.isEmpty()) // unknown W*** frame - this normally shouldn't happen map.unsupportedData().append(frameID()); else diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index 4313bcc6..a130d003 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -116,7 +116,7 @@ Frame *Frame::createTextualFrame(const String &key, const StringList &values) // { // check if the key is contained in the key<=>frameID mapping ByteVector frameID = keyToFrameID(key); - if(!frameID.isNull()) { + if(!frameID.isEmpty()) { // Apple proprietary WFED (Podcast URL) is in fact a text frame. if(frameID[0] == 'T' || frameID == "WFED"){ // text frame TextIdentificationFrame *frame = new TextIdentificationFrame(frameID, String::UTF8); @@ -364,7 +364,7 @@ String::Type Frame::checkTextEncoding(const StringList &fields, String::Type enc return checkEncoding(fields, encoding, header()->version()); } -static const TagLib::uint frameTranslationSize = 51; +static const size_t frameTranslationSize = 51; static const char *frameTranslation[][2] = { // Text information frames { "TALB", "ALBUM"}, @@ -436,41 +436,20 @@ static const char *frameTranslation[][2] = { { "WFED", "PODCASTURL" }, }; -static const TagLib::uint txxxFrameTranslationSize = 8; +static const size_t txxxFrameTranslationSize = 8; static const char *txxxFrameTranslation[][2] = { - { "MusicBrainz Album Id", "MUSICBRAINZ_ALBUMID" }, - { "MusicBrainz Artist Id", "MUSICBRAINZ_ARTISTID" }, - { "MusicBrainz Album Artist Id", "MUSICBRAINZ_ALBUMARTISTID" }, - { "MusicBrainz Release Group Id", "MUSICBRAINZ_RELEASEGROUPID" }, - { "MusicBrainz Work Id", "MUSICBRAINZ_WORKID" }, - { "Acoustid Id", "ACOUSTID_ID" }, - { "Acoustid Fingerprint", "ACOUSTID_FINGERPRINT" }, - { "MusicIP PUID", "MUSICIP_PUID" }, + { "MUSICBRAINZ ALBUM ID", "MUSICBRAINZ_ALBUMID" }, + { "MUSICBRAINZ ARTIST ID", "MUSICBRAINZ_ARTISTID" }, + { "MUSICBRAINZ ALBUM ARTIST ID", "MUSICBRAINZ_ALBUMARTISTID" }, + { "MUSICBRAINZ RELEASE GROUP ID", "MUSICBRAINZ_RELEASEGROUPID" }, + { "MUSICBRAINZ WORK ID", "MUSICBRAINZ_WORKID" }, + { "ACOUSTID ID", "ACOUSTID_ID" }, + { "ACOUSTID FINGERPRINT", "ACOUSTID_FINGERPRINT" }, + { "MUSICIP PUID", "MUSICIP_PUID" }, }; -Map &idMap() -{ - static Map m; - if(m.isEmpty()) - for(size_t i = 0; i < frameTranslationSize; ++i) - m[frameTranslation[i][0]] = frameTranslation[i][1]; - return m; -} - -Map &txxxMap() -{ - static Map m; - if(m.isEmpty()) { - for(size_t i = 0; i < txxxFrameTranslationSize; ++i) { - String key = String(txxxFrameTranslation[i][0]).upper(); - m[key] = txxxFrameTranslation[i][1]; - } - } - return m; -} - // list of deprecated frames and their successors -static const TagLib::uint deprecatedFramesSize = 4; +static const size_t deprecatedFramesSize = 4; static const char *deprecatedFrames[][2] = { {"TRDA", "TDRC"}, // 2.3 -> 2.4 (http://en.wikipedia.org/wiki/ID3) {"TDAT", "TDRC"}, // 2.3 -> 2.4 @@ -478,53 +457,49 @@ static const char *deprecatedFrames[][2] = { {"TIME", "TDRC"}, // 2.3 -> 2.4 }; -Map &deprecationMap() -{ - static Map depMap; - if(depMap.isEmpty()) - for(TagLib::uint i = 0; i < deprecatedFramesSize; ++i) - depMap[deprecatedFrames[i][0]] = deprecatedFrames[i][1]; - return depMap; -} - String Frame::frameIDToKey(const ByteVector &id) { - Map &m = idMap(); - if(m.contains(id)) - return m[id]; - if(deprecationMap().contains(id)) - return m[deprecationMap()[id]]; - return String::null; + ByteVector id24 = id; + for(size_t i = 0; i < deprecatedFramesSize; ++i) { + if(id24 == deprecatedFrames[i][0]) { + id24 = deprecatedFrames[i][1]; + break; + } + } + for(size_t i = 0; i < frameTranslationSize; ++i) { + if(id24 == frameTranslation[i][0]) + return frameTranslation[i][1]; + } + return String(); } ByteVector Frame::keyToFrameID(const String &s) { - static Map m; - if(m.isEmpty()) - for(size_t i = 0; i < frameTranslationSize; ++i) - m[frameTranslation[i][1]] = frameTranslation[i][0]; - if(m.contains(s.upper())) - return m[s]; - return ByteVector::null; + const String key = s.upper(); + for(size_t i = 0; i < frameTranslationSize; ++i) { + if(key == frameTranslation[i][1]) + return frameTranslation[i][0]; + } + return ByteVector(); } String Frame::txxxToKey(const String &description) { - Map &m = txxxMap(); - String d = description.upper(); - if(m.contains(d)) - return m[d]; + const String d = description.upper(); + for(size_t i = 0; i < txxxFrameTranslationSize; ++i) { + if(d == txxxFrameTranslation[i][0]) + return txxxFrameTranslation[i][1]; + } return d; } String Frame::keyToTXXX(const String &s) { - static Map m; - if(m.isEmpty()) - for(size_t i = 0; i < txxxFrameTranslationSize; ++i) - m[txxxFrameTranslation[i][1]] = txxxFrameTranslation[i][0]; - if(m.contains(s.upper())) - return m[s]; + const String key = s.upper(); + for(size_t i = 0; i < txxxFrameTranslationSize; ++i) { + if(key == txxxFrameTranslation[i][1]) + return txxxFrameTranslation[i][0]; + } return s; } @@ -560,7 +535,6 @@ PropertyMap Frame::asProperties() const void Frame::splitProperties(const PropertyMap &original, PropertyMap &singleFrameProperties, PropertyMap &tiplProperties, PropertyMap &tmclProperties) { - singleFrameProperties.clear(); tiplProperties.clear(); tmclProperties.clear(); diff --git a/taglib/mpeg/id3v2/id3v2frame.h b/taglib/mpeg/id3v2/id3v2frame.h index 3771f4f0..ae1e2473 100644 --- a/taglib/mpeg/id3v2/id3v2frame.h +++ b/taglib/mpeg/id3v2/id3v2frame.h @@ -264,13 +264,13 @@ namespace TagLib { /*! * Returns an appropriate ID3 frame ID for the given free-form tag key. This method - * will return ByteVector::null if no specialized translation is found. + * will return an empty ByteVector if no specialized translation is found. */ static ByteVector keyToFrameID(const String &); /*! * Returns a free-form tag name for the given ID3 frame ID. Note that this does not work - * for general frame IDs such as TXXX or WXXX; in such a case String::null is returned. + * for general frame IDs such as TXXX or WXXX; in such a case an empty string is returned. */ static String frameIDToKey(const ByteVector &); From 539d95127775f8160185d3232853cce9c96ee552 Mon Sep 17 00:00:00 2001 From: Tsuda Kageyu Date: Thu, 19 Nov 2015 10:52:46 +0900 Subject: [PATCH 69/69] Avoid using ByteVector::null where an empty vector is required. ByteVector::null is not necessarily be empty or remains the same instance. Using it in a public header may lead to a linkage error. --- taglib/asf/asffile.cpp | 8 ++++---- taglib/asf/asfpicture.cpp | 3 ++- taglib/mp4/mp4tag.cpp | 2 +- taglib/mpeg/id3v2/id3v2frame.cpp | 2 +- taglib/ogg/oggfile.cpp | 4 ++-- taglib/riff/rifffile.cpp | 4 ++-- taglib/toolkit/tbytevectorlist.cpp | 2 +- taglib/toolkit/tbytevectorstream.cpp | 2 +- taglib/toolkit/tfile.cpp | 8 ++++---- taglib/toolkit/tfile.h | 4 ++-- taglib/toolkit/tfilestream.cpp | 4 ++-- taglib/toolkit/tstring.cpp | 4 ++-- tests/test_apetag.cpp | 10 +++++----- tests/test_bytevector.cpp | 2 +- tests/test_bytevectorstream.cpp | 2 +- tests/test_id3v2.cpp | 2 +- 16 files changed, 32 insertions(+), 31 deletions(-) diff --git a/taglib/asf/asffile.cpp b/taglib/asf/asffile.cpp index e8a68d81..02fe83c3 100644 --- a/taglib/asf/asffile.cpp +++ b/taglib/asf/asffile.cpp @@ -196,7 +196,7 @@ void ASF::File::FilePrivate::BaseObject::parse(ASF::File *file, unsigned int siz if(size > 24 && size <= (unsigned int)(file->length())) data = file->readBlock(size - 24); else - data = ByteVector::null; + data = ByteVector(); } ByteVector ASF::File::FilePrivate::BaseObject::render(ASF::File * /*file*/) @@ -312,7 +312,7 @@ ByteVector ASF::File::FilePrivate::ExtendedContentDescriptionObject::render(ASF: { data.clear(); data.append(ByteVector::fromShort(attributeData.size(), false)); - data.append(attributeData.toByteVector(ByteVector::null)); + data.append(attributeData.toByteVector("")); return BaseObject::render(file); } @@ -336,7 +336,7 @@ ByteVector ASF::File::FilePrivate::MetadataObject::render(ASF::File *file) { data.clear(); data.append(ByteVector::fromShort(attributeData.size(), false)); - data.append(attributeData.toByteVector(ByteVector::null)); + data.append(attributeData.toByteVector("")); return BaseObject::render(file); } @@ -360,7 +360,7 @@ ByteVector ASF::File::FilePrivate::MetadataLibraryObject::render(ASF::File *file { data.clear(); data.append(ByteVector::fromShort(attributeData.size(), false)); - data.append(attributeData.toByteVector(ByteVector::null)); + data.append(attributeData.toByteVector("")); return BaseObject::render(file); } diff --git a/taglib/asf/asfpicture.cpp b/taglib/asf/asfpicture.cpp index cdf6e758..f772052f 100644 --- a/taglib/asf/asfpicture.cpp +++ b/taglib/asf/asfpicture.cpp @@ -132,7 +132,8 @@ ASF::Picture& ASF::Picture::operator=(const ASF::Picture& other) ByteVector ASF::Picture::render() const { if(!isValid()) - return ByteVector::null; + return ByteVector(); + return ByteVector((char)d->type) + ByteVector::fromUInt(d->picture.size(), false) + diff --git a/taglib/mp4/mp4tag.cpp b/taglib/mp4/mp4tag.cpp index 52f66906..47c802ce 100644 --- a/taglib/mp4/mp4tag.cpp +++ b/taglib/mp4/mp4tag.cpp @@ -428,7 +428,7 @@ MP4::Tag::renderFreeForm(const String &name, const MP4::Item &item) const StringList header = StringList::split(name, ":"); if(header.size() != 3) { debug("MP4: Invalid free-form item name \"" + name + "\""); - return ByteVector::null; + return ByteVector(); } ByteVector data; data.append(renderAtom("mean", ByteVector::fromUInt(0) + header[1].data(String::UTF8))); diff --git a/taglib/mpeg/id3v2/id3v2frame.cpp b/taglib/mpeg/id3v2/id3v2frame.cpp index a130d003..91eec328 100644 --- a/taglib/mpeg/id3v2/id3v2frame.cpp +++ b/taglib/mpeg/id3v2/id3v2frame.cpp @@ -170,7 +170,7 @@ ByteVector Frame::frameID() const if(d->header) return d->header->frameID(); else - return ByteVector::null; + return ByteVector(); } TagLib::uint Frame::size() const diff --git a/taglib/ogg/oggfile.cpp b/taglib/ogg/oggfile.cpp index dfd81ec6..82781412 100644 --- a/taglib/ogg/oggfile.cpp +++ b/taglib/ogg/oggfile.cpp @@ -92,7 +92,7 @@ ByteVector Ogg::File::packet(uint i) while(d->packetToPageMap.size() <= i) { if(!nextPage()) { debug("Ogg::File::packet() -- Could not find the requested packet."); - return ByteVector::null; + return ByteVector(); } } @@ -125,7 +125,7 @@ ByteVector Ogg::File::packet(uint i) if(pageIndex == d->pages.size()) { if(!nextPage()) { debug("Ogg::File::packet() -- Could not find the requested packet."); - return ByteVector::null; + return ByteVector(); } } d->currentPacketPage = d->pages[pageIndex]; diff --git a/taglib/riff/rifffile.cpp b/taglib/riff/rifffile.cpp index efd4f642..7d427a46 100644 --- a/taglib/riff/rifffile.cpp +++ b/taglib/riff/rifffile.cpp @@ -117,7 +117,7 @@ TagLib::uint RIFF::File::chunkPadding(uint i) const ByteVector RIFF::File::chunkName(uint i) const { if(i >= chunkCount()) - return ByteVector::null; + return ByteVector(); return d->chunks[i].name; } @@ -125,7 +125,7 @@ ByteVector RIFF::File::chunkName(uint i) const ByteVector RIFF::File::chunkData(uint i) { if(i >= chunkCount()) - return ByteVector::null; + return ByteVector(); seek(d->chunks[i].offset); return readBlock(d->chunks[i].size); diff --git a/taglib/toolkit/tbytevectorlist.cpp b/taglib/toolkit/tbytevectorlist.cpp index 7ea893f1..40e088d8 100644 --- a/taglib/toolkit/tbytevectorlist.cpp +++ b/taglib/toolkit/tbytevectorlist.cpp @@ -55,7 +55,7 @@ ByteVectorList ByteVectorList::split(const ByteVector &v, const ByteVector &patt if(offset - previousOffset >= 1) l.append(v.mid(previousOffset, offset - previousOffset)); else - l.append(ByteVector::null); + l.append(ByteVector()); previousOffset = offset + pattern.size(); } diff --git a/taglib/toolkit/tbytevectorstream.cpp b/taglib/toolkit/tbytevectorstream.cpp index dc480a26..230f7c99 100644 --- a/taglib/toolkit/tbytevectorstream.cpp +++ b/taglib/toolkit/tbytevectorstream.cpp @@ -71,7 +71,7 @@ FileName ByteVectorStream::name() const ByteVector ByteVectorStream::readBlock(ulong length) { if(length == 0) - return ByteVector::null; + return ByteVector(); ByteVector v = d->data.mid(d->position, length); d->position += v.size(); diff --git a/taglib/toolkit/tfile.cpp b/taglib/toolkit/tfile.cpp index a01c3460..2fca7445 100644 --- a/taglib/toolkit/tfile.cpp +++ b/taglib/toolkit/tfile.cpp @@ -289,7 +289,7 @@ long File::find(const ByteVector &pattern, long fromOffset, const ByteVector &be } } - if(!before.isNull() && beforePreviousPartialMatch >= 0 && int(bufferSize()) > beforePreviousPartialMatch) { + if(!before.isEmpty() && beforePreviousPartialMatch >= 0 && int(bufferSize()) > beforePreviousPartialMatch) { const int beforeOffset = (bufferSize() - beforePreviousPartialMatch); if(buffer.containsAt(before, 0, beforeOffset)) { seek(originalPosition); @@ -305,7 +305,7 @@ long File::find(const ByteVector &pattern, long fromOffset, const ByteVector &be return bufferOffset + location; } - if(!before.isNull() && buffer.find(before) >= 0) { + if(!before.isEmpty() && buffer.find(before) >= 0) { seek(originalPosition); return -1; } @@ -314,7 +314,7 @@ long File::find(const ByteVector &pattern, long fromOffset, const ByteVector &be previousPartialMatch = buffer.endsWithPartialMatch(pattern); - if(!before.isNull()) + if(!before.isEmpty()) beforePreviousPartialMatch = buffer.endsWithPartialMatch(before); bufferOffset += bufferSize(); @@ -387,7 +387,7 @@ long File::rfind(const ByteVector &pattern, long fromOffset, const ByteVector &b return bufferOffset + location; } - if(!before.isNull() && buffer.find(before) >= 0) { + if(!before.isEmpty() && buffer.find(before) >= 0) { seek(originalPosition); return -1; } diff --git a/taglib/toolkit/tfile.h b/taglib/toolkit/tfile.h index fe4efcba..77840684 100644 --- a/taglib/toolkit/tfile.h +++ b/taglib/toolkit/tfile.h @@ -165,7 +165,7 @@ namespace TagLib { */ long find(const ByteVector &pattern, long fromOffset = 0, - const ByteVector &before = ByteVector::null); + const ByteVector &before = ByteVector()); /*! * Returns the offset in the file that \a pattern occurs at or -1 if it can @@ -181,7 +181,7 @@ namespace TagLib { */ long rfind(const ByteVector &pattern, long fromOffset = 0, - const ByteVector &before = ByteVector::null); + const ByteVector &before = ByteVector()); /*! * Insert \a data at position \a start in the file overwriting \a replace diff --git a/taglib/toolkit/tfilestream.cpp b/taglib/toolkit/tfilestream.cpp index 65f37d52..0ced6c79 100644 --- a/taglib/toolkit/tfilestream.cpp +++ b/taglib/toolkit/tfilestream.cpp @@ -176,11 +176,11 @@ ByteVector FileStream::readBlock(ulong length) { if(!isOpen()) { debug("FileStream::readBlock() -- invalid file."); - return ByteVector::null; + return ByteVector(); } if(length == 0) - return ByteVector::null; + return ByteVector(); const ulong streamLength = static_cast(FileStream::length()); if(length > bufferSize() && length > streamLength) diff --git a/taglib/toolkit/tstring.cpp b/taglib/toolkit/tstring.cpp index d623d237..479b4069 100644 --- a/taglib/toolkit/tstring.cpp +++ b/taglib/toolkit/tstring.cpp @@ -426,7 +426,7 @@ ByteVector String::data(Type t) const return v; } else { - return ByteVector::null; + return ByteVector(); } case UTF16: { @@ -472,7 +472,7 @@ ByteVector String::data(Type t) const default: { debug("String::data() - Invalid Type value."); - return ByteVector::null; + return ByteVector(); } } } diff --git a/tests/test_apetag.cpp b/tests/test_apetag.cpp index 1a66cdd5..2f027958 100644 --- a/tests/test_apetag.cpp +++ b/tests/test_apetag.cpp @@ -98,22 +98,22 @@ public: CPPUNIT_ASSERT(unsuccessful.contains("A")); CPPUNIT_ASSERT(unsuccessful.contains("MP+")); } - + void testTextBinary() { APE::Item item = APE::Item("DUMMY", "Test Text"); CPPUNIT_ASSERT_EQUAL(String("Test Text"), item.toString()); - CPPUNIT_ASSERT_EQUAL(ByteVector::null, item.binaryData()); - + CPPUNIT_ASSERT_EQUAL(ByteVector(), item.binaryData()); + ByteVector data("Test Data"); item.setBinaryData(data); CPPUNIT_ASSERT(item.values().isEmpty()); CPPUNIT_ASSERT_EQUAL(String::null, item.toString()); CPPUNIT_ASSERT_EQUAL(data, item.binaryData()); - + item.setValue("Test Text 2"); CPPUNIT_ASSERT_EQUAL(String("Test Text 2"), item.toString()); - CPPUNIT_ASSERT_EQUAL(ByteVector::null, item.binaryData()); + CPPUNIT_ASSERT_EQUAL(ByteVector(), item.binaryData()); } }; diff --git a/tests/test_bytevector.cpp b/tests/test_bytevector.cpp index 0ab68921..b9e31f87 100644 --- a/tests/test_bytevector.cpp +++ b/tests/test_bytevector.cpp @@ -128,7 +128,7 @@ public: i.clear(); CPPUNIT_ASSERT(i.isEmpty()); - CPPUNIT_ASSERT(!i.isNull()); + CPPUNIT_ASSERT(!i.isNull()); // deprecated, but worth it to check. } void testFind1() diff --git a/tests/test_bytevectorstream.cpp b/tests/test_bytevectorstream.cpp index b0fec90b..7f6df152 100644 --- a/tests/test_bytevectorstream.cpp +++ b/tests/test_bytevectorstream.cpp @@ -56,7 +56,7 @@ public: CPPUNIT_ASSERT_EQUAL(ByteVector("a"), stream.readBlock(1)); CPPUNIT_ASSERT_EQUAL(ByteVector("bc"), stream.readBlock(2)); CPPUNIT_ASSERT_EQUAL(ByteVector("d"), stream.readBlock(3)); - CPPUNIT_ASSERT_EQUAL(ByteVector::null, stream.readBlock(3)); + CPPUNIT_ASSERT_EQUAL(ByteVector(""), stream.readBlock(3)); } void testRemoveBlock() diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 5088841f..036ab2c7 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -38,7 +38,7 @@ class PublicFrame : public ID3v2::Frame { return ID3v2::Frame::readStringField(data, encoding, positon); } virtual String toString() const { return String::null; } virtual void parseFields(const ByteVector &) {} - virtual ByteVector renderFields() const { return ByteVector::null; } + virtual ByteVector renderFields() const { return ByteVector(); } }; class TestID3v2 : public CppUnit::TestFixture