From ecd7eeee7403daa92089adcdff66897235e1c7d8 Mon Sep 17 00:00:00 2001 From: Dmitry Tsarevich Date: Thu, 23 May 2024 23:39:01 +0300 Subject: [PATCH] all: Try to make mp3 tags reading crossplatform --- APETag.cpp | 2 +- CMakeLists.txt | 2 ++ CMakeSettings.json | 22 +++++++++++++ MPAException.cpp | 24 +++----------- MPAException.h | 1 - MPAFile.cpp | 5 ++- MPAFileStream.cpp | 75 +++++++++++++++++++++++++++++--------------- MPAFileStream.h | 6 ++-- MPAFrame.cpp | 5 ++- MPAHeader.cpp | 5 ++- MPEGAudioInfoDlg.cpp | 3 +- Platform.cpp | 33 +++++++++++++++++++ Platform.h | 19 +++++++++++ Tag.h | 11 ++++--- Tags.cpp | 3 +- stdafx.h | 13 +++++++- 16 files changed, 163 insertions(+), 66 deletions(-) create mode 100644 CMakeSettings.json create mode 100644 Platform.cpp create mode 100644 Platform.h diff --git a/APETag.cpp b/APETag.cpp index 5711b6a..79138ed 100644 --- a/APETag.cpp +++ b/APETag.cpp @@ -47,7 +47,7 @@ CAPETag::CAPETag(CMPAStream* stream, bool is_appended, unsigned offset) // get size m_dwSize = stream->ReadLEValue(4, offset); - /*unsigned dwNumItems = */ stream->ReadLEValue(4U, offset); + m_dwNumItems = stream->ReadLEValue(4U, offset); // only valid for version 2 const unsigned flag{stream->ReadLEValue(4U, offset)}; diff --git a/CMakeLists.txt b/CMakeLists.txt index e693ce6..2fda71b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -103,6 +103,7 @@ target_sources(mpaheaderinfo "MPAHeader.cpp" "MPAStream.cpp" "MusicMatchTag.cpp" + "Platform.cpp" "stdafx.cpp" "Tag.cpp" "Tags.cpp" @@ -124,6 +125,7 @@ target_sources(mpaheaderinfo $ $ $ + $ $ $ $ diff --git a/CMakeSettings.json b/CMakeSettings.json new file mode 100644 index 0000000..ec1f497 --- /dev/null +++ b/CMakeSettings.json @@ -0,0 +1,22 @@ +{ + "configurations": [ + { + "name": "x64-Debug", + "generator": "Ninja", + "configurationType": "Debug", + "inheritEnvironments": [ "msvc_x64_x64" ], + "buildRoot": "${projectDir}\\out\\build\\${name}", + "installRoot": "${projectDir}\\out\\install\\${name}", + "cmakeCommandArgs": "-DMPA_BUILD_WITHOUT_ATL=True", + "buildCommandArgs": "", + "ctestCommandArgs": "", + "variables": [ + { + "name": "MPA_BUILD_WITHOUT_ATL", + "value": "true", + "type": "BOOL" + } + ] + } + ] +} \ No newline at end of file diff --git a/MPAException.cpp b/MPAException.cpp index cc822e5..d8ad50f 100644 --- a/MPAException.cpp +++ b/MPAException.cpp @@ -12,10 +12,9 @@ #include "stdafx.h" +#include "Platform.h" #include "MPAException.h" -#include - /// CMPAException: exception class ////////////////////////////////////////////// @@ -77,12 +76,6 @@ LPCTSTR CMPAException::m_szErrors[static_cast( constexpr unsigned MAX_ERR_LENGTH{256}; -void CMPAException::ShowError() { - LPCTSTR pErrorMsg = GetErrorDescription(); - // show error message - ::MessageBox(nullptr, pErrorMsg, _T("MPAFile Error"), MB_OK | MB_ICONERROR); -} - LPCTSTR CMPAException::GetErrorDescription() { if (!m_szErrorMsg) { m_szErrorMsg = new TCHAR[MAX_ERR_LENGTH]; @@ -105,18 +98,9 @@ LPCTSTR CMPAException::GetErrorDescription() { m_szErrors[static_cast(m_ErrorID)]); if (m_bGetLastError) { - // get error message of last system error id - LPVOID buffer = nullptr; - if (FormatMessage( - FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | - FORMAT_MESSAGE_IGNORE_INSERTS, - nullptr, GetLastError(), - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), // Default language - (LPTSTR)&buffer, 0, nullptr)) { - _tcscat_s(m_szErrorMsg, MAX_ERR_LENGTH, _T("\n")); - _tcscat_s(m_szErrorMsg, MAX_ERR_LENGTH, (LPCTSTR)buffer); - LocalFree(buffer); - } + _tcscat_s(m_szErrorMsg, MAX_ERR_LENGTH, _T("\n")); + _tcscat_s(m_szErrorMsg, MAX_ERR_LENGTH, + std::system_category().message(GetLastSystemError()).c_str()); } // make sure string is null-terminated diff --git a/MPAException.h b/MPAException.h index 2364af8..ab017b5 100644 --- a/MPAException.h +++ b/MPAException.h @@ -40,7 +40,6 @@ class CMPAException { ErrorIDs GetErrorID() const { return m_ErrorID; }; LPCTSTR GetErrorDescription(); - void ShowError(); private: ErrorIDs m_ErrorID; diff --git a/MPAFile.cpp b/MPAFile.cpp index e2551c0..79042d6 100644 --- a/MPAFile.cpp +++ b/MPAFile.cpp @@ -15,8 +15,7 @@ #include "MpaFile.h" #include "MPAException.h" - -#include +#include "Platform.h" CMPAFile::CMPAFile(LPCTSTR file) { m_pStream = new CMPAFileStream(file); @@ -141,7 +140,7 @@ CMPAFrame* CMPAFile::GetFrame(CMPAFile::GetType Type, CMPAFrame* frame, return GetFrame(GetType::Resync, frame, should_delete_old_frame, offset); } - OutputDebugString(e.GetErrorDescription()); + DumpSystemError(e.GetErrorDescription()); new_frame = nullptr; } diff --git a/MPAFileStream.cpp b/MPAFileStream.cpp index bf7d1e5..96e66bf 100644 --- a/MPAFileStream.cpp +++ b/MPAFileStream.cpp @@ -17,8 +17,6 @@ #include "MPAException.h" #include "MPAEndOfFileException.h" -#include - // 1KB is initial buffersize const unsigned CMPAFileStream::INIT_BUFFERSIZE = 1024U; @@ -26,21 +24,18 @@ CMPAFileStream::CMPAFileStream(LPCTSTR file_name) : CMPAStream(file_name), m_dwOffset(0) { // open with CreateFile (no limitation of 128byte filename length, like in // mmioOpen) - m_hFile = ::CreateFile(file_name, GENERIC_READ, FILE_SHARE_READ, nullptr, - OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr); + m_hFile = fopen(file_name, "rb"); - if (m_hFile == INVALID_HANDLE_VALUE) { - // throw error + if (!m_hFile) { throw CMPAException{CMPAException::ErrorIDs::ErrOpenFile, file_name, - _T("CreateFile"), - true}; + _T("CreateFile"), true}; } m_bMustReleaseFile = true; Init(); } -CMPAFileStream::CMPAFileStream(LPCTSTR file_name, HANDLE hFile) +CMPAFileStream::CMPAFileStream(LPCTSTR file_name, FILE* hFile) : CMPAStream(file_name), m_hFile(hFile), m_dwOffset(0) { m_bMustReleaseFile = false; Init(); @@ -58,20 +53,19 @@ CMPAFileStream::~CMPAFileStream() { delete[] m_pBuffer; // close file - if (m_bMustReleaseFile) ::CloseHandle(m_hFile); + if (m_bMustReleaseFile) { + fclose(m_hFile); + m_hFile = nullptr; + } } // set file position void CMPAFileStream::SetPosition(unsigned offset) const { - // convert from unsigned unsigned to signed 64bit long - const unsigned result = - ::SetFilePointer(m_hFile, offset, nullptr, FILE_BEGIN); + const int result = fseek(m_hFile, offset, SEEK_SET); - if (result == INVALID_SET_FILE_POINTER && GetLastError() != NO_ERROR) { - // != NO_ERROR - // throw error + if (result != 0) { throw CMPAException{CMPAException::ErrorIDs::ErrSetPosition, m_szFile, - _T("SetFilePointer"), true}; + _T("fseek"), true}; } } @@ -94,11 +88,42 @@ unsigned char* CMPAFileStream::ReadBytes(unsigned size, unsigned& offset, } unsigned CMPAFileStream::GetSize() const { - unsigned size = ::GetFileSize(m_hFile, nullptr); + const long start_pos{ftell(m_hFile)}; + if (start_pos == -1L) { + throw CMPAException{CMPAException::ErrorIDs::ErrReadFile, m_szFile, + _T("ftell"), true}; + } + + if (fseek(m_hFile, 0L, SEEK_END)) { + fseek(m_hFile, start_pos, SEEK_SET); + throw CMPAException{CMPAException::ErrorIDs::ErrReadFile, m_szFile, + _T("fseek"), true}; + } + +#ifdef _WIN32 + // ftell and _ftelli64 return the current file position. + // The value returned by ftell and _ftelli64 may not reflect the physical byte + // offset for streams opened in text mode, because text mode causes carriage + // return-line feed translation. + const long size{ftell(m_hFile)}; - if (size == INVALID_FILE_SIZE) + if (size == -1L) { throw CMPAException{CMPAException::ErrorIDs::ErrReadFile, m_szFile, - _T("GetFileSize"), true}; + _T("ftell"), true}; + } +#else + // https://wiki.sei.cmu.edu/confluence/display/c/FIO19-C.+Do+not+use+fseek()+and+ftell()+to+compute+the+size+of+a+regular+file + const long size{ftello(m_hFile)}; + + if (size == -1L) { + throw CMPAException{CMPAException::ErrorIDs::ErrReadFile, m_szFile, + _T("ftello"), true}; + } +#endif + + + if (fseek(m_hFile, start_pos, SEEK_SET)) { + } return size; } @@ -139,14 +164,14 @@ bool CMPAFileStream::FillBuffer(unsigned offset, unsigned size, // read from file, return number of bytes read unsigned CMPAFileStream::Read(void* data, unsigned offset, unsigned size) const { - DWORD bytes_read = 0; - // set position first SetPosition(offset); - if (!::ReadFile(m_hFile, data, size, &bytes_read, nullptr)) + const size_t bytes_read = fread(data, 1, size, m_hFile); + if (ferror(m_hFile)) throw CMPAException{CMPAException::ErrorIDs::ErrReadFile, m_szFile, - _T("ReadFile"), true}; + _T("fread"), true}; - return bytes_read; + // Safe as size is unsigned. + return static_cast(bytes_read); } \ No newline at end of file diff --git a/MPAFileStream.h b/MPAFileStream.h index b27d025..cbd35b9 100644 --- a/MPAFileStream.h +++ b/MPAFileStream.h @@ -13,6 +13,8 @@ #ifndef MPA_HEADER_INFO_MPA_FILE_STREAM_H_ #define MPA_HEADER_INFO_MPA_FILE_STREAM_H_ +#include // FILE + #include "MPAStream.h" using HANDLE = void*; @@ -20,14 +22,14 @@ using HANDLE = void*; class CMPAFileStream : public CMPAStream { public: explicit CMPAFileStream(LPCTSTR file_name); - CMPAFileStream(LPCTSTR file_name, HANDLE hFile); + CMPAFileStream(LPCTSTR file_name, FILE* hFile); virtual ~CMPAFileStream(); private: static const unsigned INIT_BUFFERSIZE; - HANDLE m_hFile; + FILE* m_hFile; bool m_bMustReleaseFile; // concerning read-buffer diff --git a/MPAFrame.cpp b/MPAFrame.cpp index 8310bfc..efb10c3 100644 --- a/MPAFrame.cpp +++ b/MPAFrame.cpp @@ -17,8 +17,7 @@ #include // for ceil #include "MPAEndOfFileException.h" - -#include +#include "Platform.h" // number of bits that are used for CRC check in MPEG 1 Layer 2 // (this table is invalid for Joint Stereo/Intensity Stereo) @@ -58,7 +57,7 @@ CMPAFrame::CMPAFrame(CMPAStream* stream, unsigned& offset, } catch (CMPAEndOfFileException&) { m_bIsLast = true; } catch (CMPAException&) { - OutputDebugString(_T("Didn't find subsequent frame")); + DumpSystemError(_T("Didn't find subsequent frame")); // if (e->GetErrorID() == CMPAException::NoFrameInTolerance } } diff --git a/MPAHeader.cpp b/MPAHeader.cpp index 85d9d66..7dab19e 100644 --- a/MPAHeader.cpp +++ b/MPAHeader.cpp @@ -16,8 +16,7 @@ #include "MPAException.h" #include "MPAEndOfFileException.h" #include "MPAStream.h" - -#include +#include "Platform.h" // static variables LPCTSTR CMPAHeader::m_szLayers[] = {_T("Layer I"), _T("Layer II"), @@ -276,7 +275,7 @@ CMPAHeader::CMPAHeader(CMPAStream* stream, unsigned& offset, errors is made upon the value of bExactOffset */ catch (const CMPAException&) { - OutputDebugString(_T("Exception at construction of MPAHeader.")); + DumpSystemError(_T("Exception at construction of MPAHeader.")); if (is_exact_offset) throw; } diff --git a/MPEGAudioInfoDlg.cpp b/MPEGAudioInfoDlg.cpp index 50c4026..0e76495 100644 --- a/MPEGAudioInfoDlg.cpp +++ b/MPEGAudioInfoDlg.cpp @@ -5,6 +5,7 @@ #include "MPEGAudioInfo.h" #include "MPEGAudioInfoDlg.h" #include "AboutDlg.h" +#include "Platform.h" #include ".\mpegaudioinfodlg.h" @@ -302,7 +303,7 @@ void CMPEGAudioInfoDlg::LoadMPEGFile(LPCTSTR szFile) } catch(CMPAException& Exc) { - Exc.ShowError(); + DumpSystemError(Exc.GetErrorDescription()); m_CtrlPrevFrame.EnableWindow(false); m_CtrlNextFrame.EnableWindow(false); m_CtrlFirstFrame.EnableWindow(false); diff --git a/Platform.cpp b/Platform.cpp new file mode 100644 index 0000000..b6dd193 --- /dev/null +++ b/Platform.cpp @@ -0,0 +1,33 @@ +// Version 3, 29 June 2007 +// +// Copyright (C) 2007 Free Software Foundation, Inc. +// +// Everyone is permitted to copy and distribute verbatim copies of this license +// document, but changing it is not allowed. +// +// This version of the GNU Lesser General Public License incorporates the terms +// and conditions of version 3 of the GNU General Public License, supplemented +// by the additional permissions listed below. + +#include "stdafx.h" + +#include "Platform.h" + +#include + +int GetLastSystemError() { +#ifdef _WIN32 + return ::GetLastError(); +#else + return errno; +#endif +} + +void DumpSystemError(const char* error) { +#ifdef _WIN32 + ::MessageBox(NULL, error, _T("MPAFile Error"), + MB_OK | MB_ICONERROR); +#else + fprintf(stderr, "mp3: %s\n", error); +#endif +} \ No newline at end of file diff --git a/Platform.h b/Platform.h new file mode 100644 index 0000000..955ea31 --- /dev/null +++ b/Platform.h @@ -0,0 +1,19 @@ +// GNU LESSER GENERAL PUBLIC LICENSE +// Version 3, 29 June 2007 +// +// Copyright (C) 2007 Free Software Foundation, Inc. +// +// Everyone is permitted to copy and distribute verbatim copies of this license +// document, but changing it is not allowed. +// +// This version of the GNU Lesser General Public License incorporates the terms +// and conditions of version 3 of the GNU General Public License, supplemented +// by the additional permissions listed below. + +#ifndef MPA_HEADER_INFO_PLATFORM_H_ +#define MPA_HEADER_INFO_PLATFORM_H_ + +int GetLastSystemError(); +void DumpSystemError(const char *error); + +#endif // !MPA_HEADER_INFO_PLATFORM_H_ \ No newline at end of file diff --git a/Tag.h b/Tag.h index a0e727b..aa18395 100644 --- a/Tag.h +++ b/Tag.h @@ -31,11 +31,12 @@ class CTag { CTag(CMPAStream* stream, LPCTSTR szName, bool is_appended, unsigned offset = 0, unsigned dwSize = 0); - unsigned m_dwOffset; // beginning of tag - unsigned m_dwSize; // size of tag - bool m_bAppended; // true if at the end of file - float m_fVersion; // format x.yz - LPTSTR m_szName; // name of tag + unsigned m_dwOffset; // beginning of tag + unsigned m_dwSize; // size of tag + unsigned m_dwNumItems; // size of tag + bool m_bAppended; // true if at the end of file + float m_fVersion; // format x.yz + LPTSTR m_szName; // name of tag void SetVersion(unsigned char bVersion1, unsigned char bVersion2 = 0, unsigned char bVersion3 = 0); }; diff --git a/Tags.cpp b/Tags.cpp index 121f871..394bae3 100644 --- a/Tags.cpp +++ b/Tags.cpp @@ -20,6 +20,7 @@ #include "Lyrics3Tag.h" #include "APETag.h" #include "MusicMatchTag.h" +#include "Platform.h" const CTags::FindTagFunctionPtr CTags::m_appendedTagFactories[] = { (CTags::FindTagFunctionPtr)&CID3V1Tag::FindTag, @@ -76,7 +77,7 @@ bool CTags::FindTag(FindTagFunctionPtr find_tag, CMPAStream* stream, return true; } } catch (CMPAException& ex) { - ex.ShowError(); + DumpSystemError(ex.GetErrorDescription()); } return false; diff --git a/stdafx.h b/stdafx.h index 9f224ba..9aa102d 100644 --- a/stdafx.h +++ b/stdafx.h @@ -29,16 +29,27 @@ typedef const char *PCTSTR, *LPCTSTR; #endif /* UNICODE */ +// We are standard compliant. +#define _CRT_SECURE_NO_WARNINGS + // dimhotepus: Add missed headers. +#ifdef _WIN32 #include // _T +#else +#define _T(x) __T(x) +#define _TEXT(x) __T(x) +#endif + #include // malloc -#include // assert #include // assert +#include // errno #include // sprintf_s #include // atoi #include +#include + #define CString std::string #define MPA_USE_STRING_FOR_CSTRING 1