diff --git a/server/src/modules/file-write/file-rename.service.test.ts b/server/src/modules/file-write/file-rename.service.test.ts index 0d04796ad..24f311193 100644 --- a/server/src/modules/file-write/file-rename.service.test.ts +++ b/server/src/modules/file-write/file-rename.service.test.ts @@ -1841,10 +1841,13 @@ describe('FileRenameService', () => { if (path.toString() === '/library/Frank Herbert/cover.jpg') return Promise.resolve(undefined); return Promise.reject(Object.assign(new Error('missing'), { code: 'ENOENT' })); }); + const warn = vi.spyOn(service['logger'], 'warn').mockImplementation(() => undefined); const result = await service.performRename(5, 12); expect(result.status).toBe('skipped'); expect(result.reason).toBe('target path already exists on disk'); + // The book's own file is free; the log must name the companion that collided. + expect(warn).toHaveBeenCalledWith(expect.stringContaining('newPath="/library/Frank Herbert/cover.jpg"')); }); describe('cancelPendingRename', () => { diff --git a/server/src/modules/file-write/file-rename.service.ts b/server/src/modules/file-write/file-rename.service.ts index 1686f049c..7d9588467 100644 --- a/server/src/modules/file-write/file-rename.service.ts +++ b/server/src/modules/file-write/file-rename.service.ts @@ -218,7 +218,7 @@ export class FileRenameService implements OnModuleDestroy { if (existingTargetPath) { const reason = 'target path already exists on disk'; this.logger.warn( - `[${FILE_RENAME_EVENT}] [end] bookId=${bookId} userId=${userId} durationMs=${Date.now() - startedAt} status=skipped reason="${sanitizeLogValue(reason)}" newPath="${sanitizeLogValue(newAbsolutePath)}" - rename skipped: target already exists on disk`, + `[${FILE_RENAME_EVENT}] [end] bookId=${bookId} userId=${userId} durationMs=${Date.now() - startedAt} status=skipped reason="${sanitizeLogValue(reason)}" newPath="${sanitizeLogValue(existingTargetPath)}" - rename skipped: target already exists on disk`, ); await this.notifyFailure(userId, bookId, `File rename skipped: ${reason}.`, suppressNotification); return { status: 'skipped', reason, oldPath: currentAbsolutePath, newPath: newAbsolutePath, durationMs: Date.now() - startedAt }; diff --git a/server/src/modules/file-write/formats/audio/audio-metadata-embedder.test.ts b/server/src/modules/file-write/formats/audio/audio-metadata-embedder.test.ts index a2ad64c6f..1448f46d4 100644 --- a/server/src/modules/file-write/formats/audio/audio-metadata-embedder.test.ts +++ b/server/src/modules/file-write/formats/audio/audio-metadata-embedder.test.ts @@ -174,9 +174,14 @@ describe('AudioMetadataEmbedder', () => { expect(probeCall[2]).toEqual({ timeout: 30_000 }); const ffmpegArgs = execFileMock.mock.calls.find(([bin]) => bin !== '/opt/bin/ffprobe')![1] as string[]; expect(ffmpegArgs).not.toContain('use_metadata_tags'); + expect(ffmpegArgs).not.toContain('-f'); }); - function mockProbedVideoStreams(stdout: string): void { + function mockProbedStreams(streams: Array<{ codec_type: string; codec_name?: string }>): void { + mockProbeOutput(JSON.stringify({ streams })); + } + + function mockProbeOutput(stdout: string): void { execFileMock.mockImplementation( (bin: string, _args: string[], _options: unknown, callback: (error: Error | null, result: { stdout: string; stderr: string }) => void) => { callback(null, { stdout: bin === 'ffprobe' ? stdout : '', stderr: '' }); @@ -184,6 +189,10 @@ describe('AudioMetadataEmbedder', () => { ); } + function ffmpegArgs(): string[] { + return execFileMock.mock.calls.find(([bin]) => bin === 'ffmpeg')![1] as string[]; + } + function mapArgs(args: string[]): string[] { return args.flatMap((arg, index) => (arg === '-map' ? [args[index + 1]!] : [])); } @@ -266,7 +275,10 @@ describe('AudioMetadataEmbedder', () => { }); it('adds freeform atoms when keeping existing art in an MP4', async () => { - mockProbedVideoStreams('0\n'); + mockProbedStreams([ + { codec_type: 'audio', codec_name: 'aac' }, + { codec_type: 'video', codec_name: 'mjpeg' }, + ]); const embedder = new AudioMetadataEmbedder(); await embedder.embedMetadata('/books/audio/book.m4a', 'm4a', { coverBytes: null, metadata: [{ key: 'publisher', value: 'Ace' }] }); @@ -277,15 +289,134 @@ describe('AudioMetadataEmbedder', () => { }); it('leaves extended tags to FFmpeg when it writes QuickTime metadata keys', async () => { - mockProbedVideoStreams(''); + mockProbedStreams([{ codec_type: 'audio', codec_name: 'aac' }]); const embedder = new AudioMetadataEmbedder(); await embedder.embedMetadata('/books/audio/book.m4b', 'm4b', { coverBytes: null, metadata: [{ key: 'subtitle', value: 'Book One' }] }); - expect(execFileMock.mock.calls.find(([bin]) => bin === 'ffmpeg')![1]).toContain('use_metadata_tags'); + expect(ffmpegArgs()).toContain('use_metadata_tags'); expect(mocks.writeMp4FreeformTags).not.toHaveBeenCalled(); }); + describe('MP4 output muxer', () => { + it.each(['mp3', 'mp2', 'opus', 'flac', 'eac3'])('writes %s audio with the mp4 muxer, which the ipod muxer rejects', (codec) => { + expect(testing.outputMuxer('m4b', [{ codecType: 'audio', codecName: codec }])).toBe('mp4'); + expect(testing.outputMuxer('m4a', [{ codecType: 'audio', codecName: codec }])).toBe('mp4'); + }); + + it.each(['aac', 'alac', 'ac3'])('leaves %s audio to the ipod muxer FFmpeg picks for the file name', (codec) => { + expect( + testing.outputMuxer('m4b', [ + { codecType: 'audio', codecName: codec }, + { codecType: 'video', codecName: 'mjpeg' }, + { codecType: 'data', codecName: 'bin_data' }, + ]), + ).toBeUndefined(); + }); + + it('uses the mp4 muxer when any audio stream is one the ipod muxer rejects', () => { + expect( + testing.outputMuxer('m4b', [ + { codecType: 'audio', codecName: 'aac' }, + { codecType: 'audio', codecName: 'mp3' }, + ]), + ).toBe('mp4'); + }); + + it('uses the mp4 muxer for audio FFmpeg could not name', () => { + expect(testing.outputMuxer('m4b', [{ codecType: 'audio', codecName: null }])).toBe('mp4'); + }); + + it('leaves the muxer alone when the streams are unknown or the format is not MP4', () => { + expect(testing.outputMuxer('m4b', null)).toBeUndefined(); + expect(testing.outputMuxer('mp3', [{ codecType: 'audio', codecName: 'mp3' }])).toBeUndefined(); + expect(testing.outputMuxer('flac', [{ codecType: 'audio', codecName: 'flac' }])).toBeUndefined(); + }); + + it('names the muxer immediately before the output path', () => { + const args = testing.buildFfmpegArgs('/a.m4b', '/c.jpg', '/t.m4b', 'm4b', [{ key: 'album', value: 'Book' }], { muxer: 'mp4' }); + + expect(args.slice(-3)).toEqual(['-f', 'mp4', '/t.m4b']); + expect(testing.buildFfmpegArgs('/a.m4b', '/c.jpg', '/t.m4b', 'm4b', [])).not.toContain('-f'); + }); + + it('probes an MP4 being given a new cover and writes its MP3 audio with the mp4 muxer', async () => { + mockProbedStreams([ + { codec_type: 'audio', codec_name: 'mp3' }, + { codec_type: 'data', codec_name: 'bin_data' }, + { codec_type: 'video', codec_name: 'mjpeg' }, + ]); + const embedder = new AudioMetadataEmbedder(); + + await embedder.embedMetadata('/books/audio/book.m4b', 'm4b', { + coverBytes: Buffer.from('raw-cover'), + metadata: [{ key: 'subtitle', value: 'Book One' }], + }); + + expect(execFileMock).toHaveBeenCalledWith( + 'ffprobe', + ['-v', 'error', '-show_entries', 'stream=codec_type,codec_name', '-of', 'json', '/books/audio/book.m4b'], + { timeout: 30_000 }, + expect.any(Function), + ); + expect(ffmpegArgs().slice(-3)).toEqual(['-f', 'mp4', '/books/audio/.bookorbit-write-fixed-id.m4b']); + expect(ffmpegArgs()).not.toContain('use_metadata_tags'); + expect(mocks.writeMp4FreeformTags).toHaveBeenCalledWith( + '/books/audio/.bookorbit-write-fixed-id.m4b', + [{ name: 'SUBTITLE', value: 'Book One' }], + { + carryOverFrom: '/books/audio/book.m4b', + }, + ); + expect(replaceFileAtomicallyMock).toHaveBeenCalledWith('/books/audio/.bookorbit-write-fixed-id.m4b', '/books/audio/book.m4b'); + }); + + it('writes an artless MP3 M4B with the mp4 muxer and FFmpeg metadata keys together', async () => { + mockProbedStreams([{ codec_type: 'audio', codec_name: 'mp3' }]); + const embedder = new AudioMetadataEmbedder(); + + await embedder.embedMetadata('/books/audio/book.m4b', 'm4b', { coverBytes: null, metadata: [{ key: 'subtitle', value: 'Book One' }] }); + + expect(ffmpegArgs()).toContain('use_metadata_tags'); + expect(ffmpegArgs().slice(-3)).toEqual(['-f', 'mp4', '/books/audio/.bookorbit-write-fixed-id.m4b']); + expect(mocks.writeMp4FreeformTags).not.toHaveBeenCalled(); + }); + + it('keeps the ipod muxer for an AAC M4B', async () => { + mockProbedStreams([ + { codec_type: 'audio', codec_name: 'aac' }, + { codec_type: 'video', codec_name: 'mjpeg' }, + ]); + const embedder = new AudioMetadataEmbedder(); + + await embedder.embedMetadata('/books/audio/book.m4b', 'm4b', { coverBytes: Buffer.from('raw-cover'), metadata: [] }); + + expect(ffmpegArgs()).not.toContain('-f'); + }); + + it.each([ + ['not JSON', 'Invalid data found when processing input'], + ['without a stream list', '{}'], + ])('treats probe output %s as unknown streams', async (_label, stdout) => { + mockProbeOutput(stdout); + const embedder = new AudioMetadataEmbedder(); + + await embedder.embedMetadata('/books/audio/book.m4b', 'm4b', { coverBytes: null, metadata: [{ key: 'subtitle', value: 'Book One' }] }); + + expect(ffmpegArgs()).not.toContain('-f'); + expect(ffmpegArgs()).not.toContain('use_metadata_tags'); + expect(mocks.writeMp4FreeformTags).toHaveBeenCalled(); + }); + + it.each(['mp3', 'flac'])('never probes a %s file', async (format) => { + const embedder = new AudioMetadataEmbedder(); + + await embedder.embedMetadata(`/books/audio/book.${format}`, format, { coverBytes: null, metadata: [] }); + + expect(execFileMock.mock.calls.map(([bin]) => bin)).toEqual(['ffmpeg']); + }); + }); + it.each(['mp3', 'flac'])('never edits MP4 atoms in %s files', async (format) => { const embedder = new AudioMetadataEmbedder(); diff --git a/server/src/modules/file-write/formats/audio/audio-metadata-embedder.ts b/server/src/modules/file-write/formats/audio/audio-metadata-embedder.ts index 7ed50b1a6..c735fae57 100644 --- a/server/src/modules/file-write/formats/audio/audio-metadata-embedder.ts +++ b/server/src/modules/file-write/formats/audio/audio-metadata-embedder.ts @@ -14,6 +14,13 @@ const execFile = promisify(execFileCallback); const FFMPEG_OUTPUT_MAX_BUFFER_BYTES = 10 * 1024 * 1024; const FFMPEG_TIMEOUT_MS = 60_000; const FFPROBE_TIMEOUT_MS = 30_000; +/** The only audio FFmpeg's ipod muxer accepts; an MP4 can also carry MP3, MP2, Opus, FLAC and E-AC-3. */ +const IPOD_MUXER_AUDIO_CODECS = new Set(['aac', 'alac', 'ac3']); + +interface ProbedStream { + codecType: string | null; + codecName: string | null; +} export interface AudioMetadataArg { key: string; @@ -44,12 +51,14 @@ export class AudioMetadataEmbedder { if (coverPath && input.coverBytes) { await writeFile(coverPath, await this.normalizedCover(input.coverBytes)); } - const useMp4MetadataTags = await shouldUseMp4MetadataTags(filePath, format, coverPath); - await execFile(resolveFfmpegPath(), buildFfmpegArgs(filePath, coverPath, tempPath, format, input.metadata, { useMp4MetadataTags }), { + const streams = isMp4Format(format) ? await probeStreams(filePath) : null; + const useMp4MetadataTags = shouldUseMp4MetadataTags(format, coverPath, streams); + const muxer = outputMuxer(format, streams); + await execFile(resolveFfmpegPath(), buildFfmpegArgs(filePath, coverPath, tempPath, format, input.metadata, { useMp4MetadataTags, muxer }), { maxBuffer: FFMPEG_OUTPUT_MAX_BUFFER_BYTES, timeout: FFMPEG_TIMEOUT_MS, }); - if (usesMp4MetadataTags(format) && !useMp4MetadataTags) { + if (isMp4Format(format) && !useMp4MetadataTags) { await writeMp4FreeformTags(tempPath, mp4FreeformTags(input.metadata), { carryOverFrom: filePath }); } await replaceFileAtomically(tempPath, filePath); @@ -80,7 +89,7 @@ function buildFfmpegArgs( tempPath: string, format: string, metadata: AudioMetadataArg[], - options?: { useMp4MetadataTags?: boolean }, + options?: { useMp4MetadataTags?: boolean; muxer?: string }, ): string[] { const args = ['-v', 'error', '-y', '-i', filePath]; @@ -102,7 +111,7 @@ function buildFfmpegArgs( args.push(entry.specifier ? `-metadata:${entry.specifier}` : '-metadata', `${entry.key}=${entry.value}`); } - const useMp4MetadataTags = options?.useMp4MetadataTags ?? (usesMp4MetadataTags(format) && !coverPath); + const useMp4MetadataTags = options?.useMp4MetadataTags ?? (isMp4Format(format) && !coverPath); if (useMp4MetadataTags) { // ffmpeg drops MP4 covr artwork when use_metadata_tags is combined with an attached picture stream. args.push('-movflags', 'use_metadata_tags'); @@ -112,6 +121,10 @@ function buildFfmpegArgs( args.push('-disposition:v:0', 'attached_pic', '-metadata:s:v:0', 'title=Album cover', '-metadata:s:v:0', 'comment=Cover (front)'); } + if (options?.muxer) { + args.push('-f', options.muxer); + } + args.push(tempPath); return args; @@ -124,26 +137,43 @@ function mp4FreeformTags(metadata: AudioMetadataArg[]): Mp4FreeformTag[] { .map((entry) => ({ name: entry.key.toUpperCase(), value: entry.value })); } -function usesMp4MetadataTags(format: string): boolean { +function isMp4Format(format: string): boolean { const normalized = format.toLowerCase(); return normalized === FORMAT_M4B || normalized === FORMAT_M4A; } -async function shouldUseMp4MetadataTags(filePath: string, format: string, coverPath: string | null): Promise { - if (!usesMp4MetadataTags(format) || coverPath) return false; - return !(await hasVideoStream(filePath)); +/** An unprobed file is treated as having art, which FFmpeg's iTunes mode would drop. */ +function shouldUseMp4MetadataTags(format: string, coverPath: string | null, streams: ProbedStream[] | null): boolean { + if (!isMp4Format(format) || coverPath || !streams) return false; + return !streams.some((stream) => stream.codecType === 'video'); } -async function hasVideoStream(filePath: string): Promise { +/** + * FFmpeg picks the ipod muxer for an .m4b or .m4a name, and it rejects audio such as the MP3 an + * Audiobookshelf merge stream-copies into an M4B. The mp4 muxer takes it; every other file keeps + * the ipod muxer and its iTunes file type. + */ +function outputMuxer(format: string, streams: ProbedStream[] | null): string | undefined { + if (!isMp4Format(format) || !streams) return undefined; + const ipodRejectsAudio = streams.some((stream) => stream.codecType === 'audio' && !IPOD_MUXER_AUDIO_CODECS.has(stream.codecName ?? '')); + return ipodRejectsAudio ? 'mp4' : undefined; +} + +async function probeStreams(filePath: string): Promise { try { const { stdout } = await execFile( resolveFfprobePath(), - ['-v', 'error', '-select_streams', 'v', '-show_entries', 'stream=index', '-of', 'csv=p=0', filePath], + ['-v', 'error', '-show_entries', 'stream=codec_type,codec_name', '-of', 'json', filePath], { timeout: FFPROBE_TIMEOUT_MS }, ); - return String(stdout).trim().length > 0; + const parsed = JSON.parse(String(stdout)) as { streams?: Array<{ codec_type?: unknown; codec_name?: unknown }> }; + if (!Array.isArray(parsed.streams)) return null; + return parsed.streams.map((stream) => ({ + codecType: typeof stream.codec_type === 'string' ? stream.codec_type : null, + codecName: typeof stream.codec_name === 'string' ? stream.codec_name : null, + })); } catch { - return true; + return null; } } @@ -169,4 +199,5 @@ async function removeFileIfPresent(path: string): Promise { export const testing = { buildFfmpegArgs, + outputMuxer, }; diff --git a/server/src/modules/file-write/formats/audio/audio-metadata-roundtrip.test.ts b/server/src/modules/file-write/formats/audio/audio-metadata-roundtrip.test.ts index df1545858..1ce8a7d92 100644 --- a/server/src/modules/file-write/formats/audio/audio-metadata-roundtrip.test.ts +++ b/server/src/modules/file-write/formats/audio/audio-metadata-roundtrip.test.ts @@ -344,7 +344,111 @@ describe('audio metadata round-trip', () => { expect(await extractArt(filePath)).toEqual(artBefore); }); - async function generateChapteredAudioFixture(filePath: string, coverPath: string): Promise { + // Audiobookshelf's merge stream-copies MP3 tracks into an MP4, leaving MP3 audio the ipod muxer rejects. + describe.each([ + ['m4b', M4bAudioFormatWriter], + ['m4a', M4aAudioFormatWriter], + ] as const)('a %s whose audio is MP3', (format, Writer) => { + it('takes new tags and a replacement cover, keeping the MP3 audio and the chapters', async () => { + const filePath = join(tempDir, `merged.${format}`); + const coverPath = join(tempDir, 'cover.jpg'); + await generateCoverFixture(coverPath); + await generateChapteredAudioFixture(filePath, coverPath, 'libmp3lame'); + + const writer = new Writer(new AudioMetadataEmbedder()); + await expect( + writer.write( + filePath, + { + title: 'Merged Title', + subtitle: 'Merged Subtitle', + seriesName: 'Merged Series', + seriesIndex: '3', + coverBytes: await readFile(coverPath), + }, + { dryRun: false, fieldMask: createBookWriteFieldMask() }, + ), + ).resolves.toEqual(expect.objectContaining({ status: 'success' })); + + const streams = await probeStreams(filePath); + expect(streams.find((stream) => stream.codec_type === 'audio')?.codec_name).toBe('mp3'); + expect(streams.find((stream) => stream.codec_type === 'video')?.disposition?.attached_pic).toBe(1); + expect(await probeChapterTitles(filePath)).toEqual(['One', 'Two']); + + vi.resetModules(); + const { extractAudioMetadata } = await import('../../../metadata/extractors/audio.extractor'); + const extracted = await extractAudioMetadata(filePath); + expect(extracted).toEqual( + expect.objectContaining({ title: 'Merged Title', subtitle: 'Merged Subtitle', seriesName: 'Merged Series', seriesIndex: '3' }), + ); + expect(extracted.coverBytes?.length).toBeGreaterThan(0); + }); + + it('takes new tags without a cover, keeping its chapters and existing art', async () => { + const filePath = join(tempDir, `merged.${format}`); + const coverPath = join(tempDir, 'cover.jpg'); + await generateCoverFixture(coverPath); + await generateChapteredAudioFixture(filePath, coverPath, 'libmp3lame'); + const artBefore = await extractArt(filePath); + + const writer = new Writer(new AudioMetadataEmbedder()); + await expect( + writer.write( + filePath, + { title: 'Tag Only', subtitle: 'Tag Only Subtitle', coverBytes: null }, + { dryRun: false, fieldMask: createBookWriteFieldMask() }, + ), + ).resolves.toEqual(expect.objectContaining({ status: 'success' })); + + expect(await probeChapterTitles(filePath)).toEqual(['One', 'Two']); + expect(await extractArt(filePath)).toEqual(artBefore); + expect(await probeFormatTags(filePath)).toEqual(expect.objectContaining({ title: 'Tag Only', subtitle: 'Tag Only Subtitle' })); + }); + + it('takes new tags when it carries no art at all', async () => { + const filePath = join(tempDir, `bare.${format}`); + await execFile(ffmpegPath, [ + '-v', + 'error', + '-y', + '-f', + 'lavfi', + '-i', + 'sine=frequency=1000:duration=0.2', + '-c:a', + 'libmp3lame', + '-f', + 'mp4', + filePath, + ]); + + const writer = new Writer(new AudioMetadataEmbedder()); + await expect( + writer.write(filePath, { title: 'Bare Title', subtitle: 'Bare Subtitle' }, { dryRun: false, fieldMask: createBookWriteFieldMask() }), + ).resolves.toEqual(expect.objectContaining({ status: 'success' })); + + expect(await probeFormatTags(filePath)).toEqual(expect.objectContaining({ title: 'Bare Title', subtitle: 'Bare Subtitle' })); + expect((await probeStreams(filePath)).find((stream) => stream.codec_type === 'audio')?.codec_name).toBe('mp3'); + }); + }); + + it('keeps the iTunes file type when rewriting an AAC m4b', async () => { + const filePath = join(tempDir, 'itunes.m4b'); + const coverPath = join(tempDir, 'cover.jpg'); + await generateCoverFixture(coverPath); + await generateChapteredAudioFixture(filePath, coverPath, 'aac'); + + const writer = new M4bAudioFormatWriter(new AudioMetadataEmbedder()); + await writer.write( + filePath, + { title: 'Branded', coverBytes: await readFile(coverPath) }, + { dryRun: false, fieldMask: createBookWriteFieldMask() }, + ); + + expect((await probeFormatTags(filePath)).major_brand?.trim()).toBe('M4A'); + }); + + async function generateChapteredAudioFixture(filePath: string, coverPath: string, audioCodec = 'aac'): Promise { const chaptersPath = join(tempDir, 'chapters.txt'); await writeFile( chaptersPath, @@ -371,7 +475,7 @@ describe('audio metadata round-trip', () => { '-map_chapters', '2', '-c:a', - 'aac', + audioCodec, '-b:a', '64k', '-c:v',