all: Try to make mp3 tags reading crossplatform

APETag.cpp

@@ -29,6 +29,7 @@ CAPETag* CAPETag::FindTag(CMPAStream* stream, bool is_appended,
 
   const unsigned char* buffer{stream->ReadBytes(8U, offset, false)};
 
+  // APEv2 only.
   if (memcmp("APETAGEX", buffer, 8U) == 0)
     return new CAPETag{stream, is_appended, offset};
 
@@ -37,6 +38,7 @@ CAPETag* CAPETag::FindTag(CMPAStream* stream, bool is_appended,
 
 CAPETag::CAPETag(CMPAStream* stream, bool is_appended, unsigned offset)
     : CTag{stream, _T("APE"), is_appended, offset} {
+  // skip APETAGEX
   offset += 8U;
 
   const unsigned version{stream->ReadLEValue(4U, offset)};
@@ -47,7 +49,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)};
@@ -63,7 +65,7 @@ CAPETag::CAPETag(CMPAStream* stream, bool is_appended, unsigned offset)
   }
 
   if (is_header) m_dwSize += 32U;  // add header
-  if (is_appended) m_dwOffset -= (m_dwSize - 32);
+  if (is_appended || is_footer) m_dwOffset -= (m_dwSize - 32);
 }
 
 CAPETag::~CAPETag() {}

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
     $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/MPAHeader.h>
     $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/MPAStream.h>
     $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/MusicMatchTag.h>
+    $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/Platform.h>
     $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/Tag.h>
     $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/Tags.h>
     $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/VBRHeader.h>
@@ -142,6 +144,7 @@ if (MPA_BUILD_WITHOUT_ATL)
   )
 endif(MPA_BUILD_WITHOUT_ATL)
 
+target_compile_features(mpaheaderinfo PRIVATE cxx_std_17)
 set_target_properties(mpaheaderinfo
   PROPERTIES VERSION ${PROJECT_VERSION} SOVERSION ${PROJECT_VERSION_MAJOR})
 

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"
+        }
+      ]
+    }
+  ]
+}

ID3V2Tag.cpp

@@ -68,7 +68,7 @@ unsigned CID3V2Tag::GetSynchsafeInteger(unsigned value) {
 
   unsigned result{0U};
 
-  for (int n = 0; n < sizeof(unsigned); n++) {
+  for (size_t n = 0; n < sizeof(unsigned); n++) {
     result = (result << 7U) | ((value & mask) >> 24U);
     value <<= 8U;
   }

LameTag.cpp

@@ -38,9 +38,9 @@ CLAMETag::CLAMETag(CMPAStream* stream, bool is_appended, unsigned offset)
 
   CString strVersion = CString{reinterpret_cast<const char*>(buffer) + 4, 4};
 #ifndef MPA_USE_STRING_FOR_CSTRING
-  m_fVersion = (float)_tstof(strVersion);
+  m_fVersion = (float)atof(strVersion);
 #else
-  m_fVersion = (float)_tstof(strVersion.c_str());
+  m_fVersion = (float)atof(strVersion.c_str());
 #endif
 
   // LAME prior to 3.90 writes only a 20 byte encoder string
@@ -72,7 +72,7 @@ CLAMETag::CLAMETag(CMPAStream* stream, bool is_appended, unsigned offset)
     m_bRevision = info_and_vbr & 0xF0;
     // invalid value
     if (m_bRevision == 15U) {
-      throw std::exception("Incorrect revision (15)");
+      throw std::out_of_range{"Incorrect revision (15)"};
     }
 
     // VBR info in 4 LSB

MPAException.cpp

@@ -12,10 +12,9 @@
 
 #include "stdafx.h"
 
+#include "Platform.h"
 #include "MPAException.h"
 
-#include <Windows.h>
-
 /// CMPAException: exception class
 //////////////////////////////////////////////
 
@@ -24,20 +23,20 @@ CMPAException::CMPAException(ErrorIDs ErrorID, LPCTSTR szFile,
     : m_ErrorID(ErrorID),
       m_bGetLastError(bGetLastError),
       m_szErrorMsg(nullptr) {
-  m_szFile = _tcsdup(szFile);
-  m_szFunction = _tcsdup(szFunction);
+  m_szFile = strdup(szFile);
+  m_szFunction = strdup(szFunction);
 }
 
 // copy constructor (necessary for exception throwing without pointers)
 CMPAException::CMPAException(const CMPAException& Source) {
   m_ErrorID = Source.m_ErrorID;
   m_bGetLastError = Source.m_bGetLastError;
-  m_szFile = _tcsdup(Source.m_szFile);
-  m_szFunction = _tcsdup(Source.m_szFunction);
+  m_szFile = strdup(Source.m_szFile);
+  m_szFunction = strdup(Source.m_szFunction);
   if (Source.m_szErrorMsg) {
     const size_t maxSize = strlen(Source.m_szErrorMsg) + 1;
     m_szErrorMsg = new char[maxSize];
-    _tcscpy_s(m_szErrorMsg, maxSize, Source.m_szErrorMsg);
+    strcpy(m_szErrorMsg, Source.m_szErrorMsg);
   } else {
     m_szErrorMsg = nullptr;
   }
@@ -77,48 +76,19 @@ LPCTSTR CMPAException::m_szErrors[static_cast<int>(
 
 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];
+    m_szErrorMsg = new char[MAX_ERR_LENGTH];
     m_szErrorMsg[0] = '\0';
 
-    TCHAR help[MAX_ERR_LENGTH];
-
-    // this is not buffer-overflow-proof!
-    if (m_szFunction) {
-      _stprintf_s(help, MAX_ERR_LENGTH, _T("%s: "), m_szFunction);
-      _tcscat_s(m_szErrorMsg, MAX_ERR_LENGTH, help);
-    }
-
-    if (m_szFile) {
-      _stprintf_s(help, MAX_ERR_LENGTH, _T("'%s'\n"), m_szFile);
-      _tcscat_s(m_szErrorMsg, MAX_ERR_LENGTH, help);
-    }
-
-    _tcscat_s(m_szErrorMsg, MAX_ERR_LENGTH,
-              m_szErrors[static_cast<int>(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);
-      }
-    }
+    char help[MAX_ERR_LENGTH];
 
+    snprintf(help, MAX_ERR_LENGTH, "%s: '%s'\n%s\n%s",
+             m_szFunction ? m_szFunction : "N/A", m_szFile ? m_szFile : "N/A",
+             m_szErrors[static_cast<int>(m_ErrorID)],
+             m_bGetLastError
+                 ? std::system_category().message(GetLastSystemError()).c_str()
+                 : "N/A");
     // make sure string is null-terminated
     m_szErrorMsg[MAX_ERR_LENGTH - 1] = '\0';
   }

MPAException.h

@@ -40,7 +40,6 @@ class CMPAException {
 
   ErrorIDs GetErrorID() const { return m_ErrorID; };
   LPCTSTR GetErrorDescription();
-  void ShowError();
 
  private:
   ErrorIDs m_ErrorID;

MPAFile.cpp

@@ -12,11 +12,10 @@
 
 #include "stdafx.h"
 
-#include "MpaFile.h"
+#include "MPAFile.h"
 
 #include "MPAException.h"
-
-#include <Windows.h>
+#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;
   }
 

MPAFileStream.cpp

@@ -17,30 +17,25 @@
 #include "MPAException.h"
 #include "MPAEndOfFileException.h"
 
-#include <Windows.h>
-
 // 1KB is initial buffersize
 const unsigned CMPAFileStream::INIT_BUFFERSIZE = 1024U;
 
 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<unsigned>(bytes_read);
 }

MPAFileStream.h

@@ -13,21 +13,23 @@
 #ifndef MPA_HEADER_INFO_MPA_FILE_STREAM_H_
 #define MPA_HEADER_INFO_MPA_FILE_STREAM_H_
 
+#include <cstdio>  // FILE
+
 #include "MPAStream.h"
 
 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