diff --git a/AUTHORS b/AUTHORS index 5d9ccc60..6f4c983c 100644 --- a/AUTHORS +++ b/AUTHORS @@ -11,7 +11,7 @@ Teemu Tervo Mathias Panzenböck Mod, S3M, IT and XM metadata implementations Tsuda Kageyu - A lot of minor improvements, such as large files support. + A lot of fixes and improvements, i.e. memory copy reduction, large files support, etc. Please send all patches and questions to taglib-devel@kde.org rather than to individual developers! diff --git a/taglib/toolkit/tbytevector.cpp b/taglib/toolkit/tbytevector.cpp index d7111bda..55b66b3d 100644 --- a/taglib/toolkit/tbytevector.cpp +++ b/taglib/toolkit/tbytevector.cpp @@ -717,9 +717,11 @@ size_t ByteVector::size() const ByteVector &ByteVector::resize(size_t size, char padding) { - detach(); - d->data->resize(d->offset + size, padding); - d->length = size; + if(size != d->length) { + detach(); + d->data->resize(d->offset + size, padding); + d->length = size; + } return *this; } diff --git a/taglib/toolkit/tfile.cpp b/taglib/toolkit/tfile.cpp index 86de3128..bcd621df 100644 --- a/taglib/toolkit/tfile.cpp +++ b/taglib/toolkit/tfile.cpp @@ -29,21 +29,14 @@ #include "tdebug.h" #include "tpropertymap.h" -#include -#include -#include - #ifdef _WIN32 -# include # include # include -# define ftruncate _chsize #else +# include # include #endif -#include - #ifndef R_OK # define R_OK 4 #endif diff --git a/taglib/toolkit/tfile.h b/taglib/toolkit/tfile.h index af55bd67..14d2fe89 100644 --- a/taglib/toolkit/tfile.h +++ b/taglib/toolkit/tfile.h @@ -208,7 +208,7 @@ namespace TagLib { bool isOpen() const; /*! - * Returns true if the file is open and readble. + * Returns true if the file is open and readable. */ bool isValid() const; diff --git a/taglib/toolkit/tfilestream.cpp b/taglib/toolkit/tfilestream.cpp index 0f120ac0..2af442e5 100644 --- a/taglib/toolkit/tfilestream.cpp +++ b/taglib/toolkit/tfilestream.cpp @@ -27,17 +27,13 @@ #include "tstring.h" #include "tdebug.h" -#include -#include - -#ifndef _WIN32 +#ifdef _WIN32 +# include +#else # include # include #endif -#include -#include - using namespace TagLib; namespace @@ -48,9 +44,10 @@ namespace typedef FileName FileNameHandle; -# define INVALID_FILE INVALID_HANDLE_VALUE + typedef HANDLE FileHandle; + const FileHandle InvalidFileHandle = INVALID_HANDLE_VALUE; - HANDLE openFile(const FileName &path, bool readOnly) + inline FileHandle openFile(const FileName &path, bool readOnly) { const DWORD access = readOnly ? GENERIC_READ : (GENERIC_READ | GENERIC_WRITE); @@ -59,34 +56,38 @@ namespace else if(!path.str().empty()) return CreateFileA(path.str().c_str(), access, FILE_SHARE_READ, NULL, OPEN_EXISTING, 0, NULL); else - return INVALID_FILE; + return InvalidFileHandle; } - size_t fread(void *ptr, size_t size, size_t nmemb, HANDLE stream) + inline void closeFile(FileHandle file) { - DWORD readLen; - if(ReadFile(stream, ptr, static_cast(size * nmemb), &readLen, NULL)) - return (readLen / size); + CloseHandle(file); + } + + inline size_t readFile(FileHandle file, ByteVector &buffer) + { + DWORD length; + if(ReadFile(file, buffer.data(), static_cast(buffer.size()), &length, NULL)) + return static_cast(length); else return 0; } - size_t fwrite(const void *ptr, size_t size, size_t nmemb, HANDLE stream) + inline size_t writeFile(FileHandle file, const ByteVector &buffer) { - DWORD writtenLen; - if(WriteFile(stream, ptr, static_cast(size * nmemb), &writtenLen, NULL)) - return (writtenLen / size); + DWORD length; + if(WriteFile(file, buffer.data(), static_cast(buffer.size()), &length, NULL)) + return static_cast(length); else return 0; } -# if _DEBUG +# ifndef NDEBUG // Convert a string in a local encoding into a UTF-16 string. - // This function should only be used to generate an error message. - // In actual use, file names in local encodings are passed to CreateFileA() - // without any conversions. + // Debugging use only. In actual use, file names in local encodings are passed to + // CreateFileA() without any conversions. String fileNameToString(const FileName &name) { @@ -110,9 +111,7 @@ namespace # endif -#else - -# define INVALID_FILE 0 +#else // _WIN32 struct FileNameHandle : public std::string { @@ -120,97 +119,82 @@ namespace operator FileName () const { return c_str(); } }; - FILE *openFile(const FileName &path, bool readOnly) + typedef FILE* FileHandle; + const FileHandle InvalidFileHandle = 0; + + inline FileHandle openFile(const FileName &path, bool readOnly) { return fopen(path, readOnly ? "rb" : "rb+"); } -#endif + inline void closeFile(FileHandle file) + { + fclose(file); + } + + inline size_t readFile(FileHandle file, ByteVector &buffer) + { + return fread(buffer.data(), sizeof(char), buffer.size(), file); + } + + inline size_t writeFile(FileHandle file, const ByteVector &buffer) + { + return fwrite(buffer.data(), sizeof(char), buffer.size(), file); + } + +#endif // _WIN32 } class FileStream::FileStreamPrivate { public: - FileStreamPrivate(const FileName &fileName, bool openReadOnly); - -#ifdef _WIN32 - - HANDLE file; - -#else - - FILE *file; - -#endif + FileStreamPrivate(const FileName &fileName) + : file(InvalidFileHandle) + , name(fileName) + , readOnly(true) + , size(0) + { + } + FileHandle file; FileNameHandle name; - bool readOnly; offset_t size; -#ifdef _WIN32 - - static const size_t bufferSize = 8196; - -#else - - static const size_t bufferSize = 1024; - -#endif + static const uint bufferSize = 1024; }; -FileStream::FileStreamPrivate::FileStreamPrivate(const FileName &fileName, bool openReadOnly) : - file(INVALID_FILE), - name(fileName), - readOnly(true), - size(0) -{ - // First try with read / write mode, if that fails, fall back to read only. - - if(!openReadOnly) - file = openFile(name, false); - - if(file != INVALID_FILE) - readOnly = false; - else - file = openFile(name, true); - - if(file == INVALID_FILE) - { -# ifdef _WIN32 - - debug("Could not open file " + fileNameToString(name)); - -# else - - debug("Could not open file " + String((const char *) name)); - -# endif - } -} - //////////////////////////////////////////////////////////////////////////////// // public members //////////////////////////////////////////////////////////////////////////////// -FileStream::FileStream(FileName file, bool openReadOnly) - : d(new FileStreamPrivate(file, openReadOnly)) +FileStream::FileStream(FileName fileName, bool openReadOnly) + : d(new FileStreamPrivate(fileName)) { + // First try with read / write mode, if that fails, fall back to read only. + + if(!openReadOnly) + d->file = openFile(fileName, false); + + if(d->file != InvalidFileHandle) + d->readOnly = false; + else + d->file = openFile(fileName, true); + + if(d->file == InvalidFileHandle) + { +# ifdef _WIN32 + debug("Could not open file " + fileNameToString(fileName)); +# else + debug("Could not open file " + String(static_cast(d->name))); +# endif + } } FileStream::~FileStream() { -#ifdef _WIN32 - if(isOpen()) - CloseHandle(d->file); - -#else - - if(isOpen()) - fclose(d->file); - -#endif + closeFile(d->file); delete d; } @@ -230,13 +214,16 @@ ByteVector FileStream::readBlock(size_t length) if(length == 0) return ByteVector::null; - if(length > FileStreamPrivate::bufferSize && static_cast(length) > FileStream::length()) - length = static_cast(FileStream::length()); + const offset_t streamLength = FileStream::length(); + if(length > FileStreamPrivate::bufferSize && static_cast(length) > streamLength) + length = static_cast(streamLength); - ByteVector v(length, 0); - const size_t count = fread(v.data(), sizeof(char), length, d->file); - v.resize(count); - return v; + ByteVector buffer(length); + + const size_t count = readFile(d->file, buffer); + buffer.resize(count); + + return buffer; } void FileStream::writeBlock(const ByteVector &data) @@ -251,7 +238,7 @@ void FileStream::writeBlock(const ByteVector &data) return; } - fwrite(data.data(), sizeof(char), data.size(), d->file); + writeFile(d->file, data); } void FileStream::insert(const ByteVector &data, offset_t start, size_t replace) @@ -272,10 +259,10 @@ void FileStream::insert(const ByteVector &data, offset_t start, size_t replace) return; } else if(data.size() < replace) { - seek(start); - writeBlock(data); - removeBlock(start + data.size(), replace - data.size()); - return; + seek(start); + writeBlock(data); + removeBlock(start + data.size(), replace - data.size()); + return; } // Woohoo! Faster (about 20%) than id3lib at last. I had to get hardcore @@ -298,40 +285,17 @@ void FileStream::insert(const ByteVector &data, offset_t start, size_t replace) offset_t readPosition = start + replace; offset_t writePosition = start; - ByteVector buffer; - ByteVector aboutToOverwrite(bufferLength, 0); - - // This is basically a special case of the loop below. Here we're just - // doing the same steps as below, but since we aren't using the same buffer - // size -- instead we're using the tag size -- this has to be handled as a - // special case. We're also using File::writeBlock() just for the tag. - // That's a bit slower than using char *'s so, we're only doing it here. - - seek(readPosition); - size_t bytesRead = fread(aboutToOverwrite.data(), sizeof(char), bufferLength, d->file); - readPosition += bufferLength; - - seek(writePosition); - writeBlock(data); - writePosition += data.size(); - - buffer = aboutToOverwrite; - - // In case we've already reached the end of file... - - buffer.resize(bytesRead); - - // Ok, here's the main loop. We want to loop until the read fails, which - // means that we hit the end of the file. - - while(!buffer.isEmpty()) { + ByteVector buffer = data; + ByteVector aboutToOverwrite(bufferLength); + while(true) + { // Seek to the current read position and read the data that we're about // to overwrite. Appropriately increment the readPosition. - + seek(readPosition); - bytesRead = fread(aboutToOverwrite.data(), sizeof(char), bufferLength, d->file); - aboutToOverwrite.resize(static_cast(bytesRead)); + const size_t bytesRead = readFile(d->file, aboutToOverwrite); + aboutToOverwrite.resize(bytesRead); readPosition += bufferLength; // Check to see if we just read the last block. We need to call clear() @@ -344,18 +308,18 @@ void FileStream::insert(const ByteVector &data, offset_t start, size_t replace) // writePosition. seek(writePosition); - fwrite(buffer.data(), sizeof(char), buffer.size(), d->file); + writeBlock(buffer); + + // We hit the end of the file. + + if(bytesRead == 0) + break; + writePosition += buffer.size(); // Make the current buffer the data that we read in the beginning. - + buffer = aboutToOverwrite; - - // Again, we need this for the last write. We don't want to write garbage - // at the end of our file, so we need to set the buffer size to the amount - // that we actually read. - - bufferLength = bytesRead; } // Clear the file size cache. @@ -376,11 +340,9 @@ void FileStream::removeBlock(offset_t start, size_t length) ByteVector buffer(bufferLength, 0); - size_t bytesRead = 1; - - while(bytesRead != 0) { + while(true) { seek(readPosition); - bytesRead = fread(buffer.data(), sizeof(char), bufferLength, d->file); + const size_t bytesRead = readFile(d->file, buffer); readPosition += bytesRead; // Check to see if we just read the last block. We need to call clear() @@ -390,9 +352,14 @@ void FileStream::removeBlock(offset_t start, size_t length) clear(); seek(writePosition); - fwrite(buffer.data(), sizeof(char), bytesRead, d->file); + writeFile(d->file, buffer); + + if(bytesRead == 0) + break; + writePosition += bytesRead; } + truncate(writePosition); } @@ -403,7 +370,7 @@ bool FileStream::readOnly() const bool FileStream::isOpen() const { - return (d->file != INVALID_FILE); + return (d->file != InvalidFileHandle); } void FileStream::seek(offset_t offset, Position p) diff --git a/taglib/toolkit/tiostream.cpp b/taglib/toolkit/tiostream.cpp index 55262954..d3cdbe1b 100644 --- a/taglib/toolkit/tiostream.cpp +++ b/taglib/toolkit/tiostream.cpp @@ -29,39 +29,41 @@ using namespace TagLib; #ifdef _WIN32 -// MSVC 2008 or later can't produce the binary for Win9x. -#if !defined(_MSC_VER) || (_MSC_VER < 1500) +# include "tstring.h" +# include "tdebug.h" +# include namespace { + // Check if the running system has CreateFileW() function. + // Windows9x systems don't have CreateFileW() or can't accept Unicode file names. - // Determines whether or not the running system is WinNT. - // In other words, whether the system supports Unicode. - - bool isWinNT() + bool supportsUnicode() { - OSVERSIONINFOA ver = {}; - ver.dwOSVersionInfoSize = sizeof(OSVERSIONINFOA); - if(GetVersionExA(&ver)) { - return (ver.dwPlatformId == VER_PLATFORM_WIN32_NT); - } - else { - return false; - } + const FARPROC p = GetProcAddress(GetModuleHandleA("kernel32"), "CreateFileW"); + return (p != NULL); } + + // Indicates whether the system supports Unicode file names. - const bool IsWinNT = isWinNT(); + const bool SystemSupportsUnicode = supportsUnicode(); // Converts a UTF-16 string into a local encoding. + // This function should only be used in Windows9x systems which don't support + // Unicode file names. - std::string unicodeToAnsi(const std::wstring &wstr) + std::string unicodeToAnsi(const wchar_t *wstr) { - const int len = WideCharToMultiByte(CP_ACP, 0, &wstr[0], -1, NULL, 0, NULL, NULL); + if(SystemSupportsUnicode) { + debug("unicodeToAnsi() - Should not be used on WinNT systems."); + } + + const int len = WideCharToMultiByte(CP_ACP, 0, wstr, -1, NULL, 0, NULL, NULL); if(len == 0) return std::string(); std::string str(len, '\0'); - WideCharToMultiByte(CP_ACP, 0, &wstr[0], -1, &str[0], len, NULL, NULL); + WideCharToMultiByte(CP_ACP, 0, wstr, -1, &str[0], len, NULL, NULL); return str; } @@ -71,20 +73,11 @@ namespace // If Win9x, converts and stores it into m_name to avoid calling Unicode version functions. FileName::FileName(const wchar_t *name) - : m_wname(IsWinNT ? name : L"") - , m_name(IsWinNT ? "" : unicodeToAnsi(name)) + : m_wname(SystemSupportsUnicode ? name : L"") + , m_name (SystemSupportsUnicode ? "" : unicodeToAnsi(name)) { } -#else - -FileName::FileName(const wchar_t *name) - : m_wname(name) -{ -} - -#endif - FileName::FileName(const char *name) : m_name(name) { @@ -92,7 +85,7 @@ FileName::FileName(const char *name) FileName::FileName(const FileName &name) : m_wname(name.m_wname) - , m_name(name.m_name) + , m_name (name.m_name) { } @@ -106,7 +99,7 @@ const std::string &FileName::str() const return m_name; } -#endif +#endif // _WIN32 //////////////////////////////////////////////////////////////////////////////// // public members diff --git a/taglib/toolkit/tstring.cpp b/taglib/toolkit/tstring.cpp index 242f11a9..36c6f12f 100644 --- a/taglib/toolkit/tstring.cpp +++ b/taglib/toolkit/tstring.cpp @@ -25,14 +25,15 @@ // This class assumes that std::basic_string has a contiguous and null-terminated buffer. -#include - #include "config.h" #include "tstring.h" #include "tdebug.h" #include "tstringlist.h" +#include +#include +#include #if defined(HAVE_MSC_BYTESWAP) # include @@ -608,31 +609,30 @@ bool String::isAscii() const String String::number(int n) // static { - if(n == 0) - return String("0"); + static const size_t BufferSize = 11; // Sufficient to store "-214748364". + static const char *Format = "%d"; - String charStack; + char buffer[BufferSize]; + int length; - bool negative = n < 0; +#if defined(HAVE_SNPRINTF) - if(negative) - n = n * -1; + length = snprintf(buffer, BufferSize, Format, n); - while(n > 0) { - int remainder = n % 10; - charStack += char(remainder + '0'); - n = (n - remainder) / 10; - } +#elif defined(HAVE_SPRINTF_S) - String s; + length = sprintf_s(buffer, Format, n); - if(negative) - s += '-'; +#else - for(int i = static_cast(charStack.d->data.size()) - 1; i >= 0; i--) - s += charStack.d->data[i]; + length = sprintf(buffer, Format, n); - return s; +#endif + + if(length > 0) + return String(buffer); + else + return String::null; } TagLib::wchar &String::operator[](size_t i) diff --git a/tests/test_id3v2.cpp b/tests/test_id3v2.cpp index 18b2143f..a3a54972 100644 --- a/tests/test_id3v2.cpp +++ b/tests/test_id3v2.cpp @@ -1,5 +1,6 @@ #include #include +#include // so evil :( #define protected public #include @@ -15,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -582,13 +584,27 @@ public: { MPEG::File f(TEST_FILE_PATH_C("compressed_id3_frame.mp3"), false); CPPUNIT_ASSERT(f.ID3v2Tag()->frameListMap().contains("APIC")); - ID3v2::AttachedPictureFrame *frame = - static_cast(f.ID3v2Tag()->frameListMap()["APIC"].front()); + +#ifdef HAVE_ZLIB + + ID3v2::AttachedPictureFrame *frame + = dynamic_cast(f.ID3v2Tag()->frameListMap()["APIC"].front()); CPPUNIT_ASSERT(frame); CPPUNIT_ASSERT_EQUAL(String("image/bmp"), frame->mimeType()); CPPUNIT_ASSERT_EQUAL(ID3v2::AttachedPictureFrame::Other, frame->type()); CPPUNIT_ASSERT_EQUAL(String(""), frame->description()); CPPUNIT_ASSERT_EQUAL(size_t(86414), frame->picture().size()); + +#else + + // Skip the test if ZLIB is not installed. + // The message "Compressed frames are currently not supported." will be displayed. + + ID3v2::UnknownFrame *frame + = dynamic_cast(f.ID3v2Tag()->frameListMap()["APIC"].front()); + CPPUNIT_ASSERT(frame); + +#endif } void testW000()