Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow FLAC files with ID3 tags to be correctly processed #58

Open
pgalbavy opened this issue Nov 21, 2019 · 9 comments
Open

Allow FLAC files with ID3 tags to be correctly processed #58

pgalbavy opened this issue Nov 21, 2019 · 9 comments

Comments

@pgalbavy
Copy link

pgalbavy commented Nov 21, 2019

Apologies for not a pull request - I am still learning git and don't want to get it too wrong.

Short patch to check if the ID3 tag is actually on a FLAC file and if so call out to the FLAC tag code. I have a number of older FLAC files with ID3 tags and I am sure others do too. Most other tools also do this.

Edit: updated to actually recover offset correctly
Edit 2: Handle EOF where ID3 block is at the end of the file, like DSF

diff --git a/id3v2.go b/id3v2.go
index 063e6cb..7449e5e 100644
--- a/id3v2.go
+++ b/id3v2.go
@@ -396,6 +396,36 @@ func ReadID3v2Tags(r io.ReadSeeker) (Metadata, error) {
                return nil, err
        }

+       // peek if this is actually a FLAC file first
+       // if it is, call the FLAC reader
+       _, err = r.Seek(int64(h.Size), io.SeekCurrent)
+       if err != nil {
+               return nil,err
+       }
+
+       b, err := readString(r, 4)
+
+       // if EOF then we have skipped an ID3 block at the end of a file, such as DSF
+       if err != io.EOF {
+               if err != nil {
+                       return nil,err
+               }
+
+               _, err = r.Seek(-4, io.SeekCurrent)
+               if err != nil {
+                       return nil,err
+               }
+
+               if (b == "fLaC") {
+                       return ReadFLACTags(r)
+               }
+       }
+
+       _, err = r.Seek(int64(-h.Size), io.SeekCurrent)
+       if err != nil {
+               return nil,err
+       }
+
        var ur io.Reader = r
        if h.Unsynchronisation {
                ur = &unsynchroniser{Reader: r}

The line offsets are from my local fork, but it should just drop in.

@wader
Copy link
Contributor

wader commented Nov 21, 2019

Hello, do you have a test file you can attach or link to somehow? so the files starts with "ID3" magic header but then there is flac file? is it an empty ID3v2 tag?

@pgalbavy
Copy link
Author

See attached, decodes correctly in mediainfo and on Windows;

01 - Lookee Here-trimmed.zip

@pgalbavy
Copy link
Author

Hang on, my "patch" causes a panic on some MP3 files. Let me debug.

@pgalbavy
Copy link
Author

Oops, forgot to reverse the seek over the ID3 block, going to edit first post

@wader
Copy link
Contributor

wader commented Nov 21, 2019

Aha sorry understood it was being weird that is it, it's just a FLAC file with a valid ID3v2 tag prepended. Both seems to have more or less the same metadata. So which one to trust? or merge with priority of one of them? dono hmm

@pgalbavy
Copy link
Author

ID3 tags on FLAC files were common before they adopted Vorbis comments (a long time ago etc.) and for a while many tools (EAC in this case, which I stopped using) create FLAC files this way. Later on, using tag editors, like mp3tag, would copy the ID3 tags to correct, modern Vorbis comments but leave the original block there for backward compatibility.

In my case I will one day remove them all but because my music is backed up "in the cloud" I would have to then re-upload multi GBs of files to sync up.

@pgalbavy
Copy link
Author

pgalbavy commented Nov 21, 2019

This also, as it stands, breaks DSF ID3 tags. Working on it.

(The reason is that the ID3 block is at the end of the file and the new lines above fall off the end of file looking for the FLAC marker)

@pgalbavy
Copy link
Author

Fixed DSF. I am reading up on git and will do proper pull requests in the future. Sorry.

@deluan
Copy link

deluan commented May 21, 2020

IMHO, I think in this case it should ignore the ID3 tag (if the FLAC has proper tags). That's what ffmpeg does. See the "Discarding ID3 tags because more suitable tags were found" message in the output below:

$ ffmpeg -i ~/Downloads/01\ -\ Lookee\ Here-trimmed.flac
ffmpeg version 4.2.2 Copyright (c) 2000-2019 the FFmpeg developers
  built with Apple clang version 11.0.0 (clang-1100.0.33.17)
  configuration: --prefix=/usr/local/Cellar/ffmpeg/4.2.2_2 --enable-shared --enable-pthreads --enable-version3 --enable-avresample --cc=clang --host-cflags= --host-ldflags= --enable-ffplay --enable-gnutls --enable-gpl --enable-libaom --enable-libbluray --enable-libmp3lame --enable-libopus --enable-librubberband --enable-libsnappy --enable-libtesseract --enable-libtheora --enable-libvidstab --enable-libvorbis --enable-libvpx --enable-libwebp --enable-libx264 --enable-libx265 --enable-libxvid --enable-lzma --enable-libfontconfig --enable-libfreetype --enable-frei0r --enable-libass --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libopenjpeg --enable-librtmp --enable-libspeex --enable-libsoxr --enable-videotoolbox --disable-libjack --disable-indev=jack
  libavutil      56. 31.100 / 56. 31.100
  libavcodec     58. 54.100 / 58. 54.100
  libavformat    58. 29.100 / 58. 29.100
  libavdevice    58.  8.100 / 58.  8.100
  libavfilter     7. 57.100 /  7. 57.100
  libavresample   4.  0.  0 /  4.  0.  0
  libswscale      5.  5.100 /  5.  5.100
  libswresample   3.  5.100 /  3.  5.100
  libpostproc    55.  5.100 / 55.  5.100
[flac @ 0x7fc5e8000000] Discarding ID3 tags because more suitable tags were found.
Input #0, flac, from '/Users/deluan/Downloads/01 - Lookee Here-trimmed.flac':
  Metadata:
    replaygain_album_gain: -2.91 dB
    replaygain_album_peak: 0.994141
    iTunNORM        :  000007A2 000007A2 00001316 00001316 00024CA8 00024CA8 00007F3F 00007F3F 00024CA8 00024CA8
    replaygain_track_gain: -2.69 dB
    replaygain_track_peak: 0.958374
    comment         : freedbID : D710940F
    CATALOGID       : NATCD38
    MEDIATYPE       : CD (Album)
    STYLE           : Tribal, Downtempo
    COUNTRY         : UK
    Title           : Lookee Here
    ARTIST          : Transglobal Underground
    ALBUM           : International Times
    GENRE           : Electronic
    album_artist    : Transglobal Underground
    disc            : 1
    DISCOGS_RELEASE_ID: 864796
    WWW             : https://www.discogs.com/Transglobal-Underground-International-Times/release/864796
    DATE            : 1994
    ORGANIZATION    : Nation Records
    track           : 1
  Duration: 00:06:23.51, start: 0.000000, bitrate: 3 kb/s
    Stream #0:0: Audio: flac, 44100 Hz, stereo, s16
    Side data:
      replaygain: track gain - -2.690000, track peak - 0.000022, album gain - -2.910000, album peak - 0.000023,
    Stream #0:1: Video: mjpeg (Progressive), yuvj444p(pc, bt470bg/unknown/unknown), 475x461 [SAR 1:1 DAR 475:461], 90k tbr, 90k tbn, 90k tbc (attached pic)
    Metadata:
      comment         : Cover (front)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants