leetreveil / musicmetadata

Streaming music metadata parser for node and the browser.
561 stars 66 forks source link

Parser for id3v1 MP3s does not detect duration #60

Open DullReferenceException opened 10 years ago

DullReferenceException commented 10 years ago

When the parser for id3v1 MP3s is selected, no duration detection is performed.

DullReferenceException commented 10 years ago

I'm not sure what the best way is to fix this, but maybe merging the ID3v1 and ID3v2 parsers would work; really, the ID3v2 parser is a parser for an entire MP3 file itself. If the ID3v1 parsing was merged into that, then we'd have the same duration detection for both.

leetreveil commented 10 years ago

Are ID3v1 audio files that common anymore? I would really like to remove support for it as it requires parsing the entire file to get to the metadata.

Fix for this probably would be to merge the logic, not sure if the container spec is the same for id3v1 and 2 though.

Pheo-Player commented 10 years ago

I'd say ID3v1 files are common enough (i.e. they sadly still exist) that support for them should be kept, especially for a widely used library like this.

For duration detection, id3v1 files should make virtually no difference: No TLEN tag exists, but that's about it. There's no duration related data to read from the metadata; you'd need always to infer the duration from somewhere else. Easy for CBR, harder for VBR, but Xing headers (https://github.com/leetreveil/musicmetadata/issues/42#issuecomment-53035173) can still exist in id3v1 files, they're not dependent on id3v2.

On the (somewhat unrelated) issue of parsing the entire file for metadata: Would it be possible to get the file size, e.g. via stat, substracting the 128 bytes of an id3v1 tag, and using that information to start reading at that exact point? There'd be no need to parse the rest of the file for the metadata, then.

heapwolf commented 8 years ago

:+1: for a fix

Borewit commented 7 years ago

MPEG parsing is not done in case a ID3v1 header is assumed to be parsed. I fixed this in music-metadata by moving the MPEG (MP3) parsing functionality from ID3v2 to a separate file and including it the ID3v1 tag parser.