From 5c44726e9c811d66d41c4e430022b73f155563c8 Mon Sep 17 00:00:00 2001 From: Fabian Zwick Date: Wed, 23 Oct 2024 23:29:54 +0200 Subject: [PATCH] Avoid reading the whole file into memory when updating ID3 tag of a file (#106) :rocket: * Save to temporary file without reading everything into memory. * Increase chunk size * Add autoreleasepool * Refactoring. * Add test case. * Fix failing tests. * Fix build on tvOS and Linux. --------- Co-authored-by: Fabian Zwick --- Source/ID3TagEditor.swift | 16 ++-- Source/Mp3/Mp3FileReader.swift | 60 +++++++++----- Source/Mp3/Mp3FileReaderFactory.swift | 6 +- Source/Mp3/Mp3FileWriter.swift | 112 +++++++++++++++++++++++++- Source/Mp3/Mp3WithID3TagBuilder.swift | 4 +- Tests/Mp3/Mp3FileReaderTest.swift | 36 +++++---- Tests/Mp3/Mp3FileWriterTest.swift | 37 +++++++++ 7 files changed, 224 insertions(+), 47 deletions(-) create mode 100644 Tests/Mp3/Mp3FileWriterTest.swift diff --git a/Source/ID3TagEditor.swift b/Source/ID3TagEditor.swift index a101f90b..c4c2e8c8 100644 --- a/Source/ID3TagEditor.swift +++ b/Source/ID3TagEditor.swift @@ -12,6 +12,7 @@ import Foundation */ public class ID3TagEditor { private let id3TagParser: ID3TagParser + private let id3TagCreator: ID3TagCreator private let mp3FileReader: Mp3FileReader private let mp3FileWriter: Mp3FileWriter private let mp3WithID3TagBuilder: Mp3WithID3TagBuilder @@ -21,9 +22,10 @@ public class ID3TagEditor { */ public init() { self.id3TagParser = ID3TagParserFactory.make() + self.id3TagCreator = ID3TagCreatorFactory.make() self.mp3FileReader = Mp3FileReaderFactory.make() self.mp3FileWriter = Mp3FileWriter() - self.mp3WithID3TagBuilder = Mp3WithID3TagBuilder(id3TagCreator: ID3TagCreatorFactory.make(), + self.mp3WithID3TagBuilder = Mp3WithID3TagBuilder(id3TagCreator: id3TagCreator, id3TagConfiguration: ID3TagConfiguration()) } @@ -38,7 +40,10 @@ public class ID3TagEditor { Could throw `CorruptedFile` if the file is corrupted. */ public func read(from path: String) throws -> ID3Tag? { - let mp3 = try mp3FileReader.readID3TagFrom(path: path) + guard let mp3 = try mp3FileReader.readID3TagFrom(path: path) else { + return nil + } + return try self.id3TagParser.parse(mp3: mp3) } @@ -68,10 +73,9 @@ public class ID3TagEditor { ID3 tag). */ public func write(tag: ID3Tag, to path: String, andSaveTo newPath: String? = nil) throws { - let mp3 = try mp3FileReader.readFileFrom(path: path) - let currentTag = try self.id3TagParser.parse(mp3: mp3) - let mp3WithId3Tag = try mp3WithID3TagBuilder.build(mp3: mp3, newId3Tag: tag, currentId3Tag: currentTag) - try mp3FileWriter.write(mp3: mp3WithId3Tag, path: newPath ?? path) + let currentId3TagData = try mp3FileReader.readID3TagFrom(path: path) + let newId3TagData = try id3TagCreator.create(id3Tag: tag) + try mp3FileWriter.write(newId3TagData: newId3TagData, currentId3TagData: currentId3TagData, fromPath: path, toPath: newPath ?? path) } /** diff --git a/Source/Mp3/Mp3FileReader.swift b/Source/Mp3/Mp3FileReader.swift index 6c376f0b..db33fd5a 100644 --- a/Source/Mp3/Mp3FileReader.swift +++ b/Source/Mp3/Mp3FileReader.swift @@ -10,11 +10,17 @@ import Foundation class Mp3FileReader { private let tagSizeParser: TagSizeParser private let id3TagConfiguration: ID3TagConfiguration + private let tagVersionParser: TagVersionParser + private let tagPresence: TagPresence init(tagSizeParser: TagSizeParser, - id3TagConfiguration: ID3TagConfiguration) { + id3TagConfiguration: ID3TagConfiguration, + tagVersionParser: TagVersionParser, + tagPresence: TagPresence) { self.tagSizeParser = tagSizeParser self.id3TagConfiguration = id3TagConfiguration + self.tagVersionParser = tagVersionParser + self.tagPresence = tagPresence } /** @@ -41,40 +47,54 @@ class Mp3FileReader { - parameter path: the path to the mp3 file - - returns: ID3 header data of the file + - returns: ID3 header data or nil, if a tag doesn't exists in the file. - - throws: Could throw `InvalidFileFormat` if an mp3 file doesn't exists at the specified path, or if the file - does not contain the entire ID3 header + - throws: Could throw `InvalidFileFormat` if an mp3 file doesn't exists at the specified path. + Could throw `CorruptedFile` if the file is corrupted. */ - func readID3TagFrom(path: String) throws -> Data { + func readID3TagFrom(path: String) throws -> Data? { let validPath = URL(fileURLWithPath: path) guard validPath.pathExtension.caseInsensitiveCompare("mp3") == ComparisonResult.orderedSame else { throw ID3TagEditorError.invalidFileFormat } - guard let inputStream = InputStream(fileAtPath: path) else { - throw ID3TagEditorError.corruptedFile + let readHandle = try FileHandle(forReadingFrom: URL(fileURLWithPath: path)) + defer { + if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) { + try? readHandle.close() + } else { + readHandle.closeFile() + } } - inputStream.open() - let headerSize = id3TagConfiguration.headerSize() - let header = try read(bytesCount: headerSize, fromStream: inputStream) - let headerData = Data(header) as NSData + let header = try read(bytesCount: headerSize, from: readHandle) - let frameSize = tagSizeParser.parse(data: headerData) - let frame = try read(bytesCount: Int(frameSize), fromStream: inputStream) + // Verify that there is a valid ID3 tag to parse the size from + let version = tagVersionParser.parse(mp3: header) + guard tagPresence.isTagPresentIn(mp3: header, version: version) else { + return nil + } - let mp3 = header + frame - return Data(mp3) + let frameSize = tagSizeParser.parse(data: header as NSData) + let frame = try read(bytesCount: Int(frameSize), from: readHandle) + + return header + frame } - private func read(bytesCount: Int, fromStream stream: InputStream) throws -> [UInt8] { - var buffer = [UInt8](repeating: 0, count: bytesCount) - let result = stream.read(&buffer, maxLength: bytesCount) - if result < bytesCount { + private func read(bytesCount: Int, from fileHandle: FileHandle) throws -> Data { + let result = try { + if #available(macOS 10.15.4, iOS 13.4, watchOS 6.2, tvOS 13.4, *) { + return try fileHandle.read(upToCount: bytesCount) + } else { + return fileHandle.readData(ofLength: bytesCount) + } + }() + + guard let result, result.count == bytesCount else { throw ID3TagEditorError.corruptedFile } - return buffer + + return result } } diff --git a/Source/Mp3/Mp3FileReaderFactory.swift b/Source/Mp3/Mp3FileReaderFactory.swift index d1a322c9..f72b5236 100644 --- a/Source/Mp3/Mp3FileReaderFactory.swift +++ b/Source/Mp3/Mp3FileReaderFactory.swift @@ -13,8 +13,10 @@ class Mp3FileReaderFactory { let tagSizeParser = ID3TagSizeParser() let id3TagConfiguration = ID3TagConfiguration() let fileReader = Mp3FileReader(tagSizeParser: tagSizeParser, - id3TagConfiguration: id3TagConfiguration) - + id3TagConfiguration: id3TagConfiguration, + tagVersionParser: ID3TagVersionParser(), + tagPresence: ID3TagPresence(id3TagConfiguration: id3TagConfiguration)) + return fileReader } } diff --git a/Source/Mp3/Mp3FileWriter.swift b/Source/Mp3/Mp3FileWriter.swift index 9e73e0d3..474b6788 100644 --- a/Source/Mp3/Mp3FileWriter.swift +++ b/Source/Mp3/Mp3FileWriter.swift @@ -8,9 +8,115 @@ import Foundation class Mp3FileWriter { - func write(mp3: Data, path: String) throws { - try eventuallyCreateIntermediatesDirectoriesFor(path: path) - try mp3.write(to: URL(fileURLWithPath: path)) + func write(newId3TagData: Data, currentId3TagData: Data?, fromPath: String, toPath: String) throws { + let validPath = URL(fileURLWithPath: toPath) + guard validPath.pathExtension.caseInsensitiveCompare("mp3") == ComparisonResult.orderedSame else { + throw ID3TagEditorError.invalidFileFormat + } + + // Create a temporary file for the new mp3 + let temporaryPath = { + if toPath != fromPath { + return toPath + } + + return FileManager.default.temporaryDirectory.appendingPathComponent("\(UUID().uuidString).mp3").path + }() + + defer { + if temporaryPath != toPath { + try? FileManager.default.removeItem(atPath: temporaryPath) + } + } + + try eventuallyCreateIntermediatesDirectoriesFor(path: temporaryPath) + try newId3TagData.write(to: URL(fileURLWithPath: temporaryPath)) + + // Create file handles + let readHandle = try FileHandle(forReadingFrom: URL(fileURLWithPath: fromPath)) + defer { + if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) { + try? readHandle.close() + } else { + readHandle.closeFile() + } + } + + let writeHandle = try FileHandle(forWritingTo: URL(fileURLWithPath: temporaryPath)) + defer { + if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) { + try? writeHandle.close() + } else { + writeHandle.closeFile() + } + } + + // Seek over the tag of the existing file, then copy the rest in chunks + if #available(macOS 10.15.4, iOS 13.4, watchOS 6.2, tvOS 13.4, *) { + try writeHandle.seekToEnd() + } else { + writeHandle.seekToEndOfFile() + } + + if let currentId3TagData = currentId3TagData { + if #available(macOS 10.15, iOS 13.0, watchOS 6.0, tvOS 13.0, *) { + try readHandle.seek(toOffset: UInt64(currentId3TagData.count)) + } else { + readHandle.seek(toFileOffset: UInt64(currentId3TagData.count)) + } + } + + var isFinished = false + while !isFinished { + let work = { + let chunk = try { + if #available(macOS 10.15.4, iOS 13.4, watchOS 6.2, tvOS 13.4, *) { + return try readHandle.read(upToCount: 131072) // 128 KB + } else { + return readHandle.readData(ofLength: 131072) // 128 KB + } + }() + + if let chunk, !chunk.isEmpty { + if #available(macOS 10.15.4, iOS 13.4, watchOS 6.2, tvOS 13.4, *) { + try writeHandle.write(contentsOf: chunk) + } else { + writeHandle.write(chunk) + } + } else { + isFinished = true + } + } + +#if canImport(ObjectiveC) + // autoreleasepool is only needed in Objective-C environment (not on Linux) + try autoreleasepool(invoking: work) +#else + try work() +#endif + } + + // Replace the file + if temporaryPath != toPath { +#if os(Linux) + // For some reason the FileManager.replaceItemAt(_:withItemAt:) doesn't work on Linux and fails with `NSFileWriteUnknownError` + let backupPath = URL(fileURLWithPath: toPath).appendingPathExtension("tmp").path + try FileManager.default.copyItem(atPath: toPath, toPath: backupPath) + defer { + try? FileManager.default.removeItem(atPath: backupPath) + } + + do { + try FileManager.default.removeItem(atPath: toPath) + try FileManager.default.copyItem(atPath: temporaryPath, toPath: toPath) + } catch { + try? FileManager.default.copyItem(atPath: backupPath, toPath: toPath) + throw error + } +#else + _ = try FileManager.default.replaceItemAt(validPath, withItemAt: URL(fileURLWithPath: temporaryPath)) +#endif + } } private func eventuallyCreateIntermediatesDirectoriesFor(path: String) throws { diff --git a/Source/Mp3/Mp3WithID3TagBuilder.swift b/Source/Mp3/Mp3WithID3TagBuilder.swift index e68f96fc..ddf9efad 100644 --- a/Source/Mp3/Mp3WithID3TagBuilder.swift +++ b/Source/Mp3/Mp3WithID3TagBuilder.swift @@ -22,7 +22,9 @@ class Mp3WithID3TagBuilder { tagSizeWithHeader = Int(validCurrentId3Tag.properties.size) + ID3TagConfiguration().headerSize() } var mp3WithTag = try id3TagCreator.create(id3Tag: newId3Tag) - mp3WithTag.append(mp3.subdata(in: tagSizeWithHeader..