search

benrr101 / node-taglib-sharp

A node.js port of mono/taglib-sharp
GNU Lesser General Public License v2.1
42 stars 11 forks source link

id3v2 frameId RGAD causes failure to read tags #90

Closed digimezzo closed 1 year ago

digimezzo commented 1 year ago

As user of my software notified me that tags could not be read from some of his files (files attached). I did some debugging to try to help out, but I'm now stuck.

This Guard in the FrameHeader constructor throws an error Argument null: id was not provided

image

And indeed, frameId seems to be undefined:

image

I did the same test in TagLib#, and where node-taglib-sharp returns undefined, TagLib# returns a frameIdentifier of type RGAD:

image

I see that frameIdentifiers.ts does not contain a frameIdentifier of type RGAD. Is it just a matter of adding it?

Songs.error.zip

benrr101 commented 1 year ago

Hi @digimezzo, thanks for the report and the investigation. I think this is actually a duplicate of #85 and you were really close to pinpointing what I think is the issue. A while back to improve the memory requirements for ID3v2 and unify the identifiers to make converting between ID3v2 versions easier, I went to a static list of identifiers from the ID3v2 docs. When reading the frames, the frame identifier is converted to a string and used as the index into the static list. Since RGAD is a non-standard identifier, pulling it up from the static list returns undefined.

Rather than allow that to throw later on, it should be creating an on-the-fly identifier and the frame factory should treat it as an UnknownFrame. I have a proposed solution for this but was waiting for the #85 reporter to test it out. Can you try out this beta version and see if it resolves the issue? If so, I can publish it as a hotfix in a couple days https://www.npmjs.com/package/node-taglib-sharp/v/5.2.1-fixid3v2nonstandardframes.1

digimezzo commented 1 year ago

@benrr101 I tried the beta version and it fixes the Argument null: id was not provided error. Thank you for the fix.

benrr101 commented 1 year ago

@digimezzo, thanks for testing! I just published the v5.2.2 release that contains the fix https://www.npmjs.com/package/node-taglib-sharp/v/5.2.2

Interestingly, the RGAD frame isn't totally non-standard, it's a legacy ReplayGain frame format. As such, I think it'd be useful to add support for reading it and optionally writing it. However, I'll close this one and track that issue on another issue.

stuartambient commented 1 year ago

I have a proposed solution for this but was waiting for the #85 reporter to test it out.

Sorry @benrr101 never got to testing.