diff --git a/go_backend/audio_metadata.go b/go_backend/audio_metadata.go index ee963fef..f632a0e7 100644 --- a/go_backend/audio_metadata.go +++ b/go_backend/audio_metadata.go @@ -53,16 +53,21 @@ type OggQuality struct { } func ReadID3Tags(filePath string) (*AudioMetadata, error) { + metadata, _, _, err := readID3TagsAndCover(filePath, false) + return metadata, err +} + +func readID3TagsAndCover(filePath string, includeCover bool) (*AudioMetadata, []byte, string, error) { file, err := os.Open(filePath) if err != nil { - return nil, err + return nil, nil, "", err } defer file.Close() metadata := &AudioMetadata{} - id3v2, err := readID3v2(file) - if err == nil && id3v2 != nil { + id3v2, cover, mime, err := readID3v2WithCover(file, includeCover) + if id3v2 != nil { metadata = id3v2 } @@ -88,80 +93,33 @@ func ReadID3Tags(filePath string) (*AudioMetadata, error) { } if metadata.Title == "" && metadata.Artist == "" { - return nil, fmt.Errorf("no ID3 tags found") + return nil, cover, mime, fmt.Errorf("no ID3 tags found") } - return metadata, nil + return metadata, cover, mime, nil } func readID3v2(file *os.File) (*AudioMetadata, error) { - file.Seek(0, io.SeekStart) - - header := make([]byte, 10) - if _, err := io.ReadFull(file, header); err != nil { - return nil, err - } - - if string(header[0:3]) != "ID3" { - return nil, fmt.Errorf("no ID3v2 header") - } - - majorVersion := header[3] - flags := header[5] - unsync := (flags & 0x80) != 0 - extendedHeader := (flags & 0x40) != 0 - footerPresent := (flags & 0x10) != 0 - - size := int(header[6])<<21 | int(header[7])<<14 | int(header[8])<<7 | int(header[9]) - - tagData := make([]byte, size) - if _, err := io.ReadFull(file, tagData); err != nil { - return nil, err - } - - if footerPresent && len(tagData) >= 10 { - footerStart := len(tagData) - 10 - if footerStart >= 0 && string(tagData[footerStart:footerStart+3]) == "3DI" { - tagData = tagData[:footerStart] - } - } - - if extendedHeader { - if skip := extendedHeaderSize(tagData, majorVersion); skip > 0 && skip < len(tagData) { - tagData = tagData[skip:] - } - } - - metadata := &AudioMetadata{} - - if majorVersion == 2 { - parseID3v22Frames(tagData, metadata, unsync) - } else { - parseID3v23Frames(tagData, metadata, majorVersion, unsync) - } - - return metadata, nil + metadata, _, _, err := readID3v2WithCover(file, false) + return metadata, err } func parseID3v22Frames(data []byte, metadata *AudioMetadata, tagUnsync bool) { - pos := 0 - for pos+6 < len(data) { - frameID := string(data[pos : pos+3]) - if frameID[0] == 0 { - break - } + parseID3Frames(data, metadata, 2, tagUnsync) +} - frameSize := int(data[pos+3])<<16 | int(data[pos+4])<<8 | int(data[pos+5]) - if frameSize <= 0 || pos+6+frameSize > len(data) { - break - } +func parseID3v23Frames(data []byte, metadata *AudioMetadata, version byte, tagUnsync bool) { + parseID3Frames(data, metadata, version, tagUnsync) +} - frameData := data[pos+6 : pos+6+frameSize] - if tagUnsync { - frameData = removeUnsync(frameData) - } - value := firstTextValue(extractTextFrame(frameData)) +func parseID3Frames(data []byte, metadata *AudioMetadata, version byte, tagUnsync bool) { + _ = walkID3Frames(bytes.NewReader(data), int64(len(data)), version, tagUnsync, nil, + func(id string, payload []byte) { applyID3Frame(metadata, version, id, payload) }) +} +func applyID3Frame(metadata *AudioMetadata, version byte, frameID string, frameData []byte) { + value := firstTextValue(extractTextFrame(frameData)) + if version == 2 { switch frameID { case "TT2": metadata.Title = value @@ -203,154 +161,70 @@ func parseID3v22Frames(data []byte, metadata *AudioMetadata, tagUnsync bool) { metadata.UPC = userValue } } - - pos += 6 + frameSize + return } -} - -func parseID3v23Frames(data []byte, metadata *AudioMetadata, version byte, tagUnsync bool) { - pos := 0 - for pos+10 < len(data) { - frameID := string(data[pos : pos+4]) - if frameID[0] == 0 { - break + switch frameID { + case "TIT2": + metadata.Title = value + case "TPE1": + metadata.Artist = value + case "TPE2": + metadata.AlbumArtist = value + case "TALB": + metadata.Album = value + case "TYER", "TDRC": + metadata.Year = value + if len(value) >= 4 { + metadata.Date = value } - - var frameSize int - if version == 4 { - frameSize = int(data[pos+4])<<21 | int(data[pos+5])<<14 | int(data[pos+6])<<7 | int(data[pos+7]) - } else { - frameSize = int(data[pos+4])<<24 | int(data[pos+5])<<16 | int(data[pos+6])<<8 | int(data[pos+7]) + case "TCON": + metadata.Genre = cleanGenre(value) + case "TRCK": + metadata.TrackNumber, metadata.TotalTracks = parseIndexPair(value) + case "TPOS": + metadata.DiscNumber, metadata.TotalDiscs = parseIndexPair(value) + case "TSRC": + metadata.ISRC = value + case "TCOM": + metadata.Composer = value + case "TPUB": + metadata.Label = value + case "TCOP": + metadata.Copyright = value + case "TCMP": + if isTruthyTagValue(value) && metadata.AlbumType == "" { + metadata.AlbumType = "compilation" } - - if frameSize <= 0 || pos+10+frameSize > len(data) { - break + case "COMM": + if v := extractLangTextFrame(frameData); v != "" { + metadata.Comment = v } - - frameData := data[pos+10 : pos+10+frameSize] - - statusFlags := data[pos+8] - _ = statusFlags - formatFlags := data[pos+9] - - if version == 3 { - const ( - id3v23FlagCompression = 0x80 - id3v23FlagEncryption = 0x40 - id3v23FlagGrouping = 0x20 - ) - if formatFlags&(id3v23FlagCompression|id3v23FlagEncryption) != 0 { - pos += 10 + frameSize - continue - } - if formatFlags&id3v23FlagGrouping != 0 { - if len(frameData) < 1 { - pos += 10 + frameSize - continue - } - frameData = frameData[1:] - } - if tagUnsync { - frameData = removeUnsync(frameData) - } - } else if version == 4 { - const ( - id3v24FlagGrouping = 0x40 - id3v24FlagCompression = 0x08 - id3v24FlagEncryption = 0x04 - id3v24FlagUnsync = 0x02 - id3v24FlagDataLen = 0x01 - ) - if formatFlags&id3v24FlagGrouping != 0 { - if len(frameData) < 1 { - pos += 10 + frameSize - continue - } - frameData = frameData[1:] - } - if formatFlags&id3v24FlagDataLen != 0 { - if len(frameData) < 4 { - pos += 10 + frameSize - continue - } - frameData = frameData[4:] - } - if formatFlags&id3v24FlagUnsync != 0 || tagUnsync { - frameData = removeUnsync(frameData) - } - if formatFlags&(id3v24FlagCompression|id3v24FlagEncryption) != 0 { - pos += 10 + frameSize - continue - } + case "USLT": + if v := extractLangTextFrame(frameData); v != "" && metadata.Lyrics == "" { + metadata.Lyrics = v } - - value := firstTextValue(extractTextFrame(frameData)) - - switch frameID { - case "TIT2": - metadata.Title = value - case "TPE1": - metadata.Artist = value - case "TPE2": - metadata.AlbumArtist = value - case "TALB": - metadata.Album = value - case "TYER", "TDRC": - metadata.Year = value - if len(value) >= 4 { - metadata.Date = value - } - case "TCON": - metadata.Genre = cleanGenre(value) - case "TRCK": - metadata.TrackNumber, metadata.TotalTracks = parseIndexPair(value) - case "TPOS": - metadata.DiscNumber, metadata.TotalDiscs = parseIndexPair(value) - case "TSRC": - metadata.ISRC = value - case "TCOM": - metadata.Composer = value - case "TPUB": - metadata.Label = value - case "TCOP": - metadata.Copyright = value - case "TCMP": - if isTruthyTagValue(value) && metadata.AlbumType == "" { - metadata.AlbumType = "compilation" - } - case "COMM": - if v := extractLangTextFrame(frameData); v != "" { - metadata.Comment = v - } - case "USLT": - if v := extractLangTextFrame(frameData); v != "" && metadata.Lyrics == "" { - metadata.Lyrics = v - } - case "TXXX": - desc, userValue := extractUserTextFrame(frameData) - if isLyricsDescription(desc) && userValue != "" && metadata.Lyrics == "" { - metadata.Lyrics = userValue - } - upperDesc := strings.ToUpper(desc) - switch upperDesc { - case "REPLAYGAIN_TRACK_GAIN": - metadata.ReplayGainTrackGain = userValue - case "REPLAYGAIN_TRACK_PEAK": - metadata.ReplayGainTrackPeak = userValue - case "REPLAYGAIN_ALBUM_GAIN": - metadata.ReplayGainAlbumGain = userValue - case "REPLAYGAIN_ALBUM_PEAK": - metadata.ReplayGainAlbumPeak = userValue - case "ITUNESADVISORY": - metadata.Explicit = isTruthyTagValue(userValue) - case "RELEASETYPE": - metadata.AlbumType = userValue - case "BARCODE", "UPC": - metadata.UPC = userValue - } + case "TXXX": + desc, userValue := extractUserTextFrame(frameData) + if isLyricsDescription(desc) && userValue != "" && metadata.Lyrics == "" { + metadata.Lyrics = userValue + } + upperDesc := strings.ToUpper(desc) + switch upperDesc { + case "REPLAYGAIN_TRACK_GAIN": + metadata.ReplayGainTrackGain = userValue + case "REPLAYGAIN_TRACK_PEAK": + metadata.ReplayGainTrackPeak = userValue + case "REPLAYGAIN_ALBUM_GAIN": + metadata.ReplayGainAlbumGain = userValue + case "REPLAYGAIN_ALBUM_PEAK": + metadata.ReplayGainAlbumPeak = userValue + case "ITUNESADVISORY": + metadata.Explicit = isTruthyTagValue(userValue) + case "RELEASETYPE": + metadata.AlbumType = userValue + case "BARCODE", "UPC": + metadata.UPC = userValue } - - pos += 10 + frameSize } } diff --git a/go_backend/audio_metadata_cover.go b/go_backend/audio_metadata_cover.go index f9d55659..3a063241 100644 --- a/go_backend/audio_metadata_cover.go +++ b/go_backend/audio_metadata_cover.go @@ -17,65 +17,13 @@ func extractMP3CoverArt(filePath string) ([]byte, string, error) { return nil, "", err } defer file.Close() - - header := make([]byte, 10) - if _, err := io.ReadFull(file, header); err != nil { + _, cover, mime, err := readID3v2WithCover(file, true) + if len(cover) > 0 { + return cover, mime, nil + } + if err != nil { return nil, "", err } - - if string(header[0:3]) != "ID3" { - return nil, "", fmt.Errorf("no ID3v2 header") - } - - majorVersion := header[3] - size := int(header[6])<<21 | int(header[7])<<14 | int(header[8])<<7 | int(header[9]) - - tagData := make([]byte, size) - if _, err := io.ReadFull(file, tagData); err != nil { - return nil, "", err - } - - pos := 0 - var frameIDLen, headerLen int - if majorVersion == 2 { - frameIDLen = 3 - headerLen = 6 - } else { - frameIDLen = 4 - headerLen = 10 - } - - for pos+headerLen < len(tagData) { - frameID := string(tagData[pos : pos+frameIDLen]) - if frameID[0] == 0 { - break - } - - var frameSize int - switch majorVersion { - case 2: - frameSize = int(tagData[pos+3])<<16 | int(tagData[pos+4])<<8 | int(tagData[pos+5]) - case 4: - frameSize = int(tagData[pos+4])<<21 | int(tagData[pos+5])<<14 | int(tagData[pos+6])<<7 | int(tagData[pos+7]) - default: - frameSize = int(tagData[pos+4])<<24 | int(tagData[pos+5])<<16 | int(tagData[pos+6])<<8 | int(tagData[pos+7]) - } - - if frameSize <= 0 || pos+headerLen+frameSize > len(tagData) { - break - } - - if (frameIDLen == 4 && frameID == "APIC") || (frameIDLen == 3 && frameID == "PIC") { - frameData := tagData[pos+headerLen : pos+headerLen+frameSize] - imageData, mimeType := parseAPICFrame(frameData, majorVersion) - if len(imageData) > 0 { - return imageData, mimeType, nil - } - } - - pos += headerLen + frameSize - } - return nil, "", fmt.Errorf("no cover art found") } diff --git a/go_backend/id3_reader.go b/go_backend/id3_reader.go new file mode 100644 index 00000000..74aa119d --- /dev/null +++ b/go_backend/id3_reader.go @@ -0,0 +1,199 @@ +package gobackend + +import ( + "bufio" + "encoding/binary" + "fmt" + "io" +) + +// A malformed frame must not allocate an entire declared tag (up to 256 MiB). +// Artwork is optional; tag-only callers seek past it without allocating it. +const maxID3FrameBytes = 32 << 20 + +func readID3v2WithCover(file io.ReadSeeker, includeCover bool) (*AudioMetadata, []byte, string, error) { + if _, err := file.Seek(0, io.SeekStart); err != nil { + return nil, nil, "", err + } + var header [10]byte + if _, err := io.ReadFull(file, header[:]); err != nil { + return nil, nil, "", err + } + if string(header[:3]) != "ID3" { + return nil, nil, "", fmt.Errorf("no ID3v2 header") + } + version, flags := header[3], header[5] + if version < 2 || version > 4 || header[6]|header[7]|header[8]|header[9] >= 128 { + return nil, nil, "", fmt.Errorf("invalid ID3 version or tag size") + } + size := int64(syncsafeToInt(header[6:10])) + end, err := file.Seek(0, io.SeekEnd) + if err != nil { + return nil, nil, "", err + } + if size > end-10 { + return nil, nil, "", io.ErrUnexpectedEOF + } + if _, err := file.Seek(10, io.SeekStart); err != nil { + return nil, nil, "", err + } + var reader io.Reader = file + // ID3v2.2/2.3 unsynchronization covers the entire tag, including headers; + // decode it as a bounded stream so large pictures still need no allocation. + if flags&0x80 != 0 && version < 4 { + reader = &id3UnsyncReader{source: bufio.NewReader(io.LimitReader(file, size))} + } + if flags&0x40 != 0 { + if version == 2 { + return nil, nil, "", fmt.Errorf("compressed ID3v2.2 tag unsupported") + } + var extended [4]byte + if _, err := io.ReadFull(reader, extended[:]); err != nil { + return nil, nil, "", err + } + length := int64(binary.BigEndian.Uint32(extended[:])) + if version == 4 { + length = int64(syncsafeToInt(extended[:])) - 4 + } + if length < 0 || length > size-4 { + return nil, nil, "", fmt.Errorf("invalid ID3 extended header") + } + if err := skipID3Bytes(reader, length); err != nil { + return nil, nil, "", err + } + size -= length + 4 + } + metadata := &AudioMetadata{} + var cover []byte + var mime string + err = walkID3Frames(reader, size, version, version == 4 && flags&0x80 != 0, func() bool { return includeCover && len(cover) == 0 }, + func(id string, data []byte) { + if id == "APIC" || id == "PIC" { + if len(cover) == 0 { + cover, mime = parseAPICFrame(data, version) + } + } else { + applyID3Frame(metadata, version, id, data) + } + }) + return metadata, cover, mime, err +} + +// walkID3Frames is shared by metadata, cover extraction and combined scanning. +// Only selected frame payloads are read; the enclosing tag bounds every seek. +func walkID3Frames(reader io.Reader, remaining int64, version byte, tagUnsync bool, wantCover func() bool, visit func(string, []byte)) error { + headerLength, idLength := 10, 4 + if version == 2 { + headerLength, idLength = 6, 3 + } + var header [10]byte + for remaining >= int64(headerLength) { + if count, err := io.ReadFull(reader, header[:headerLength]); err != nil { + // A globally unsynchronized tag can have fewer decoded bytes than its + // raw size; exhaustion between frames is normal. + if err == io.EOF { + return nil + } + if err == io.ErrUnexpectedEOF { + padding := true + for _, value := range header[:count] { + if value != 0 { + padding = false + break + } + } + if padding { + return nil + } + } + return err + } + remaining -= int64(headerLength) + if header[0] == 0 || string(header[:3]) == "3DI" { + return nil + } + id := string(header[:idLength]) + var size int64 + switch version { + case 2: + size = int64(header[3])<<16 | int64(header[4])<<8 | int64(header[5]) + case 4: + if header[4]|header[5]|header[6]|header[7] >= 128 { + return fmt.Errorf("invalid ID3 frame size") + } + size = int64(syncsafeToInt(header[4:8])) + default: + size = int64(binary.BigEndian.Uint32(header[4:8])) + } + if size <= 0 || size > remaining { + return fmt.Errorf("invalid ID3 frame bounds") + } + remaining -= size + picture := id == "APIC" || id == "PIC" + wanted := (picture && wantCover != nil && wantCover()) || (!picture && (id[0] == 'T' || id == "COMM" || id == "USLT" || id == "ULT")) + flags := byte(0) + if version != 2 { + flags = header[9] + } + unsupported := (version == 3 && flags&0xc0 != 0) || (version == 4 && flags&0x0c != 0) + if !wanted || unsupported || size > maxID3FrameBytes { + if err := skipID3Bytes(reader, size); err != nil { + return err + } + continue + } + data := make([]byte, int(size)) + if _, err := io.ReadFull(reader, data); err != nil { + return err + } + if version == 3 && flags&0x20 != 0 || version == 4 && flags&0x40 != 0 { + if len(data) < 1 { + continue + } + data = data[1:] + } + if version == 4 && flags&0x01 != 0 { + if len(data) < 4 { + continue + } + data = data[4:] + } + if tagUnsync || version == 4 && flags&0x02 != 0 { + data = removeUnsync(data) + } + visit(id, data) + } + return nil +} + +func skipID3Bytes(reader io.Reader, size int64) error { + if seeker, ok := reader.(io.Seeker); ok { + _, err := seeker.Seek(size, io.SeekCurrent) + return err + } + _, err := io.CopyN(io.Discard, reader, size) + return err +} + +type id3UnsyncReader struct { + source *bufio.Reader + afterFF bool +} + +func (reader *id3UnsyncReader) Read(output []byte) (int, error) { + count := 0 + for count < len(output) { + value, err := reader.source.ReadByte() + if err != nil { + return count, err + } + if reader.afterFF && value == 0 { + reader.afterFF = false + continue + } + reader.afterFF = value == 0xff + output[count] = value + count++ + } + return count, nil +} diff --git a/go_backend/id3_reader_test.go b/go_backend/id3_reader_test.go new file mode 100644 index 00000000..4849c70f --- /dev/null +++ b/go_backend/id3_reader_test.go @@ -0,0 +1,220 @@ +package gobackend + +import ( + "bytes" + "encoding/binary" + "fmt" + "io" + "os" + "path/filepath" + "testing" +) + +func id3ReaderTestFrame(version byte, id string, payload []byte) []byte { + if version == 2 { + out := []byte{id[0], id[1], id[2], byte(len(payload) >> 16), byte(len(payload) >> 8), byte(len(payload))} + return append(out, payload...) + } + frame := id3v23Frame(id, payload) + if version == 4 { + copy(frame[4:8], syncsafeBytes(len(payload))) + } + return frame +} + +func id3ReaderTestTag(version, flags byte, body []byte) []byte { + header := []byte{'I', 'D', '3', version, 0, flags, 0, 0, 0, 0} + copy(header[6:10], syncsafeBytes(len(body))) + return append(header, body...) +} + +func TestID3CombinedReaderVersionsExtendedHeadersAndUnsync(t *testing.T) { + cover := []byte{0xff, 0xd8, 0xff, 0xe0, 1, 2, 3} + for _, version := range []byte{2, 3, 4} { + for _, extended := range []bool{false, true} { + if version == 2 && extended { + continue + } + for _, unsync := range []bool{false, true} { + t.Run(fmt.Sprintf("v%d/extended=%v/unsync=%v", version, extended, unsync), func(t *testing.T) { + titleID, artistID, pictureID := "TIT2", "TPE1", "APIC" + picture := append([]byte{0, 'i', 'm', 'a', 'g', 'e', '/', 'j', 'p', 'e', 'g', 0, 3, 0}, cover...) + if version == 2 { + titleID, artistID, pictureID = "TT2", "TP1", "PIC" + picture = append([]byte{0, 'J', 'P', 'G', 3, 0}, cover...) + } + frames := append(id3ReaderTestFrame(version, titleID, []byte{0, 'T', 'i', 't', 'l', 'e'}), id3ReaderTestFrame(version, artistID, []byte{0, 'A', 'r', 't', 'i', 's', 't'})...) + var flags byte + if version == 4 && unsync { + picture = bytes.ReplaceAll(picture, []byte{0xff}, []byte{0xff, 0}) + flags |= 0x80 + } + frames = append(frames, id3ReaderTestFrame(version, pictureID, picture)...) + if extended { + flags |= 0x40 + prefix := []byte{0, 0, 0, 6, 0, 0} + if version == 3 { + prefix = append(prefix, 0, 0, 0, 0) + } + frames = append(prefix, frames...) + } + if version < 4 && unsync { + frames = bytes.ReplaceAll(frames, []byte{0xff}, []byte{0xff, 0}) + flags |= 0x80 + } + data := id3ReaderTestTag(version, flags, frames) + metadata, got, mime, err := readID3v2WithCover(bytes.NewReader(data), true) + if err != nil || metadata.Title != "Title" || metadata.Artist != "Artist" || !bytes.Equal(got, cover) || mime != "image/jpeg" { + t.Fatalf("metadata=%+v cover=%x mime=%s err=%v", metadata, got, mime, err) + } + path := filepath.Join(t.TempDir(), "track.mp3") + if err := os.WriteFile(path, data, 0600); err != nil { + t.Fatal(err) + } + standalone, _, err := extractMP3CoverArt(path) + if err != nil || !bytes.Equal(standalone, got) { + t.Fatalf("standalone cover=%x err=%v", standalone, err) + } + }) + } + } + } +} + +type countingID3Reader struct { + *bytes.Reader + bytesRead int +} + +func (reader *countingID3Reader) Read(out []byte) (int, error) { + count, err := reader.Reader.Read(out) + reader.bytesRead += count + return count, err +} + +func TestID3MetadataSkipsLargeCoverPayload(t *testing.T) { + picture := append([]byte{0, 'i', 'm', 'a', 'g', 'e', '/', 'j', 'p', 'e', 'g', 0, 3, 0}, bytes.Repeat([]byte{1}, 8<<20)...) + tag := buildID3v23Tag(id3TextFrame("TIT2", "Song"), id3v23Frame("APIC", picture), id3TextFrame("TSRC", "USRC17607839")) + reader := &countingID3Reader{Reader: bytes.NewReader(tag)} + metadata, cover, _, err := readID3v2WithCover(reader, false) + if err != nil || metadata.ISRC != "USRC17607839" || len(cover) != 0 { + t.Fatalf("metadata=%+v cover=%d err=%v", metadata, len(cover), err) + } + if reader.bytesRead > 1024 { + t.Fatalf("tag-only read consumed %d bytes, including artwork", reader.bytesRead) + } + reader = &countingID3Reader{Reader: bytes.NewReader(tag)} + _, cover, _, err = readID3v2WithCover(reader, true) + if err != nil || len(cover) != 8<<20 { + t.Fatalf("cover=%d err=%v", len(cover), err) + } + if reader.bytesRead != len(tag) { + t.Fatalf("combined read consumed %d bytes for %d-byte tag", reader.bytesRead, len(tag)) + } +} + +func TestID3ReaderRejectsMalformedBoundsAndSkipsUnsupportedFrames(t *testing.T) { + tag := buildID3v23Tag(id3TextFrame("TIT2", "Song")) + truncated := append([]byte{}, tag[:len(tag)-1]...) + if _, _, _, err := readID3v2WithCover(bytes.NewReader(truncated), false); err == nil { + t.Fatal("accepted truncated declared tag") + } + oversized := append([]byte{}, tag...) + binary.BigEndian.PutUint32(oversized[14:18], 1<<30) + if _, _, _, err := readID3v2WithCover(bytes.NewReader(oversized), true); err == nil { + t.Fatal("accepted frame outside tag") + } + compressed := id3TextFrame("TIT2", "unsupported") + compressed[9] = 0x80 + good := id3TextFrame("TIT2", "Song") + metadata, _, _, err := readID3v2WithCover(bytes.NewReader(buildID3v23Tag(compressed, good)), false) + if err != nil || metadata.Title != "Song" { + t.Fatalf("metadata=%+v err=%v", metadata, err) + } +} + +func TestScanMP3CombinedMetadataAndCover(t *testing.T) { + dir := t.TempDir() + cover := []byte{0xff, 0xd8, 0xff, 1, 2, 3} + picture := append([]byte{0, 'i', 'm', 'a', 'g', 'e', '/', 'j', 'p', 'e', 'g', 0, 3, 0}, cover...) + path, _ := writeTestMP3(t, dir, id3TextFrame("TIT2", "Song"), id3TextFrame("TPE1", "Artist"), id3CommentFrame("USLT", "words"), id3v23Frame("APIC", picture)) + cache := filepath.Join(dir, "covers") + for _, pass := range []string{"cold", "warm"} { + result, err := scanMP3FileWithCoverCache(path, &LibraryScanResult{FilePath: path}, "", cache, "key") + if err != nil || result.TrackName != "Song" || !result.HasLyrics || result.CoverPath == "" { + t.Fatalf("%s result=%+v err=%v", pass, result, err) + } + got, err := os.ReadFile(result.CoverPath) + if err != nil || !bytes.Equal(got, cover) { + t.Fatalf("cover=%x err=%v", got, err) + } + } +} + +func BenchmarkID3MetadataLargeCover(b *testing.B) { + picture := append([]byte{0, 'i', 'm', 'a', 'g', 'e', '/', 'j', 'p', 'e', 'g', 0, 3, 0}, bytes.Repeat([]byte{1}, 8<<20)...) + tag := buildID3v23Tag(id3TextFrame("TIT2", "Song"), id3v23Frame("APIC", picture)) + reader := bytes.NewReader(tag) + b.ReportAllocs() + b.ResetTimer() + for b.Loop() { + reader.Seek(0, io.SeekStart) + if _, _, _, err := readID3v2WithCover(reader, false); err != nil { + b.Fatal(err) + } + } +} + +func TestID3PartialTagsAndCoverSurviveMalformedTrailingFrame(t *testing.T) { + cover := []byte{0xff, 0xd8, 0xff, 1, 2, 3} + picture := append([]byte{0, 'i', 'm', 'a', 'g', 'e', '/', 'j', 'p', 'e', 'g', 0, 3, 0}, cover...) + bad := id3TextFrame("TALB", "bad") + binary.BigEndian.PutUint32(bad[4:8], 1<<30) + tag := buildID3v23Tag(id3TextFrame("TIT2", "Song"), id3v23Frame("APIC", picture), bad) + metadata, _, _, err := readID3v2WithCover(bytes.NewReader(tag), true) + if err == nil || metadata.Title != "Song" { + t.Fatalf("partial metadata=%+v err=%v", metadata, err) + } + path := filepath.Join(t.TempDir(), "song.mp3") + if err := os.WriteFile(path, tag, 0600); err != nil { + t.Fatal(err) + } + metadata, err = ReadID3Tags(path) + if err != nil || metadata.Title != "Song" { + t.Fatalf("path metadata=%+v err=%v", metadata, err) + } + got, _, err := extractMP3CoverArt(path) + if err != nil || !bytes.Equal(got, cover) { + t.Fatalf("partial cover=%x err=%v", got, err) + } +} + +func TestID3GlobalUnsyncAllowsShortDecodedPadding(t *testing.T) { + for _, version := range []byte{2, 3} { + pictureID, titleID := "APIC", "TIT2" + if version == 2 { + pictureID, titleID = "PIC", "TT2" + } + body := id3ReaderTestFrame(version, titleID, []byte{0, 'S', 'o', 'n', 'g'}) + body = append(body, id3ReaderTestFrame(version, pictureID, bytes.Repeat([]byte{0xff, 0xe0}, 20))...) + body = append(body, 0, 0, 0) + body = bytes.ReplaceAll(body, []byte{0xff}, []byte{0xff, 0}) + metadata, _, _, err := readID3v2WithCover(bytes.NewReader(id3ReaderTestTag(version, 0x80, body)), false) + if err != nil || metadata.Title != "Song" { + t.Fatalf("v%d metadata=%+v err=%v", version, metadata, err) + } + } +} + +func TestID3CombinedReaderSkipsAdditionalArtwork(t *testing.T) { + picture := append([]byte{0, 'i', 'm', 'a', 'g', 'e', '/', 'j', 'p', 'e', 'g', 0, 3, 0}, bytes.Repeat([]byte{1}, 2<<20)...) + tag := buildID3v23Tag(id3TextFrame("TIT2", "Song"), id3v23Frame("APIC", picture), id3v23Frame("APIC", picture), id3TextFrame("TSRC", "USRC17607839")) + reader := &countingID3Reader{Reader: bytes.NewReader(tag)} + metadata, cover, _, err := readID3v2WithCover(reader, true) + if err != nil || metadata.ISRC != "USRC17607839" || len(cover) != 2<<20 { + t.Fatalf("metadata=%+v cover=%d err=%v", metadata, len(cover), err) + } + if reader.bytesRead > len(picture)+1024 { + t.Fatalf("read %d bytes, including unused second cover", reader.bytesRead) + } +} diff --git a/go_backend/library_scan_formats.go b/go_backend/library_scan_formats.go index 23dade0b..f1d9ddd0 100644 --- a/go_backend/library_scan_formats.go +++ b/go_backend/library_scan_formats.go @@ -36,6 +36,8 @@ func scanAudioFileWithKnownModTimeAndDisplayNameAndCoverCacheKey(filePath, displ scanned, scanErr = scanFLACFileWithCoverCache(filePath, result, displayNameHint, coverCacheDir, coverCacheKey) } else if ext == ".m4a" || ext == ".mp4" || ext == ".aac" { scanned, scanErr = scanM4AFileWithCoverCache(filePath, result, displayNameHint, coverCacheDir, coverCacheKey) + } else if ext == ".mp3" { + scanned, scanErr = scanMP3FileWithCoverCache(filePath, result, displayNameHint, coverCacheDir, coverCacheKey) } else { if coverCacheDir != "" { coverPath, err := SaveCoverToCacheWithHintAndKey( @@ -50,8 +52,6 @@ func scanAudioFileWithKnownModTimeAndDisplayNameAndCoverCacheKey(filePath, displ } switch ext { - case ".mp3": - scanned, scanErr = scanMP3File(filePath, result, displayNameHint) case ".opus", ".ogg": scanned, scanErr = scanOggFile(filePath, result, displayNameHint) case ".ape", ".wv", ".mpc": @@ -277,7 +277,20 @@ func isLosslessLibraryFormat(format string) bool { } func scanMP3File(filePath string, result *LibraryScanResult, displayNameHint string) (*LibraryScanResult, error) { - metadata, err := ReadID3Tags(filePath) + return scanMP3FileWithCoverCache(filePath, result, displayNameHint, "", "") +} + +func scanMP3FileWithCoverCache(filePath string, result *LibraryScanResult, displayNameHint, cacheDir, cacheKey string) (*LibraryScanResult, error) { + wantCover := cacheDir != "" + if wantCover { + cacheKey = resolveLibraryCoverCacheKey(filePath, cacheKey) + result.CoverPath = existingLibraryCoverCachePath(cacheDir, cacheKey) + wantCover = result.CoverPath == "" + } + metadata, cover, mime, err := readID3TagsAndCover(filePath, wantCover) + if wantCover && len(cover) > 0 { + result.CoverPath, _ = saveLibraryCoverDataToCache(cacheDir, cacheKey, cover, mime) + } if err != nil { GoLog("[LibraryScan] ID3 read error for %s: %v\n", filePath, err) return scanFromFilename(filePath, displayNameHint, result)