benrr101 / node-taglib-sharp

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

Throwing error: `Argument null: ${name} was not provided` #85

Closed stuartambient closed 6 months ago

stuartambient commented 9 months ago

Not sure what is causing this, file tags read in 'music-metadata'. It's only happening on one file (right now) so let me know if you'd need to see the file. I'm using the properties class (ICode and IAudioCode) basically sending the properties individually to the console.

Full error:

throw new Error(`Argument null: ${name} was not provided`);                 ^

Error: Argument null: id was not provided
    at Guards.truthy (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\utils.js:86:19)
    at new Id3v2FrameHeader (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\id3v2\frames\frameHeader.js:65:24)
    at Id3v2FrameHeader.fromData (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\id3v2\frames\frameHeader.js:133:16)
    at Object.createFrame (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\id3v2\frames\frameFactory.js:88:55)
    at Id3v2Tag.parse (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\id3v2\id3v2Tag.js:1250:54)
    at Id3v2Tag.readFromStart (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\id3v2\id3v2Tag.js:1274:14)
    at Id3v2Tag.fromFileStart (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\id3v2\id3v2Tag.js:80:13)
    at Object.action (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\sandwich\startTag.js:120:50)
    at StartTagParser.read (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\sandwich\startTag.js:97:47)
    at StartTag.read (C:\Users\sambi\Documents\nodeProjs\backup-cli\node_modules\node-taglib-sharp\dist\sandwich\startTag.js:70:23)
vk22 commented 7 months ago

I also got the same error today on one group of files. Although music-metadata reading them fine. I'd like to understand the cause.

benrr101 commented 7 months ago

Just wanted to let you folks know I'm not ignoring you, just been way too busy lately. MPEG4 release is top of the list but I'll look at this as soon as that's wrapped up.

benrr101 commented 7 months ago

Ok, I think I see what the issue is here, although without seeing the file that's causing the issue I can't confirm. Based on the stack trace (thanks @stuartambient), it looks like we're trying to read a frame in that has a non-standard identifier. In this scenario we shouldn't get to the point where an exception is thrown, but generate an identifier and allow it to get processed as an UnknownFrame.

Let me write up a code change for this, but I'd also like it if one of you could try it before releasing. I'd also like to double check that the list of known identifiers is correct and the frame that's causing this issue is truly non-standard. I'll supply a bit of test code that'll generate some output that'd be useful.

benrr101 commented 7 months ago

I have a fix ready, can you replace the reference to node-taglib-sharp in your packages.json with: "node-taglib-sharp": "5.2.1-fixid3v2nonstandardframes.1"

And then if you can run this snippet replacing pathToFile with the path to your problem file and reply back with the output, I'd really appreciate it!

import * as taglib from "node-taglib-sharp";

const file = taglib.File.createFromPath(pathToFile);
console.log("Frame list:");
const id3v2Tag = <taglib.Id3v2Tag>file.getTag(taglib.TagTypes.Id3v2, false);
for (const frame of id3v2Tag.frames) {
    const v2 = frame.frameId["_version2"].toString(taglib.StringType.Latin1);
    const v3 = frame.frameId["_version3"].toString(taglib.StringType.Latin1);
    const v4 = frame.frameId["_version4"].toString(taglib.StringType.Latin1);

    console.log(`  v2: ${v2}, v3: ${v3}, v4: ${v4}`);
}
stuartambient commented 6 months ago

Thank you for this, will try to get to testing later in the week. Sorry though because I didn't mark the offending file(s). I have enough files though to test on. Will update soon!

the0neWhoKnocks commented 6 months ago

Just ran into this issue myself. In my case it seems the file was tagged with id3v2.2 tags and the frame names were padded with null bytes (ex. "TS2\u0000").

Within id3v2/frames/frameHeader.js I changed this:

- 108 | frameId = frameIdentifiers_1.FrameIdentifiers[frameName];
+ 108 | let frameName = data.subarray(0, 4).toString(byteVector_1.StringType.Latin1);
+ 109 | // frame name ends with a null byte so remove it
+ 110 | if (/\0$/.test(frameName)) { frameName = frameName.replace(/\0$/, ''); }
+ 111 | frameId = frameIdentifiers_1.FrameIdentifiers[frameName];

Still getting errors for null: id was not provided due to the version being picked up as 3 (due to the padding I assume).

Not sure if there's a way to detect null bytes and remove them before header._majorVersion = data.get(3); is set within id3v2/id3v2TagHeader.js.


While poking around I also noticed that some v2 tags were missing within id3v2/frameIdentifiers.js:

- 139 | TCMP: new FrameIdentifier("TCMP", "TCMP", undefined),
+ 139 | TCMP: new FrameIdentifier("TCMP", "TCMP", "TCP"),

- 181 | TSO2: new FrameIdentifier("TSO2", "TSO2", undefined),
+ 181 | TSO2: new FrameIdentifier("TSO2", "TSO2", "TS2"),

Tried out node-taglib-sharp@5.2.1-fixid3v2nonstandardframes.1 and I'm no longer getting errors. Also not getting the tag information (due to the version mismatch), but one thing at a time I guess.

benrr101 commented 6 months ago

@stuartambient thanks for reporting, I've published the fix as v5.2.2 https://www.npmjs.com/package/node-taglib-sharp/v/5.2.2 Let me know if there are any other issues!

@the0neWhoKnocks Thanks for taking a look at the fix! My understanding of ID3v2.2 was that it was the only version that supported 3 character frame identifiers. It also looks like TS2 and TCP were not defined frame identifiers in that version (https://web.archive.org/web/20210509100744/https://id3.org/id3v2-00). Without a spec for TS2/TCP, I can't say for sure if these are ID3v2.2 frame identifiers being padded to fit ID3v2.3 or if the null byte is part of the data of the frame. As you noted, there are some frame identifiers that were added in ID3v2.3 and ID3v2.4 that had no equivalent in ID3v2.2. I think if there's some indication that TS2/TCP frames are in widespread use (forum post, whatever), we can add them to the list. But otherwise, non-standard frames would need to be handled with the built-in mechanisms. Although I haven't fully written the documentation on this in the wiki, there is a supported way to handle unsupported frames. Take a look at this example:

import * as taglib from "node-taglib-sharp";

const ts2 = taglib.ByteVector.fromString("TS2\0", taglib.StringType.Latin1);
const frameCreator = (data: taglib.ByteVector, offset: number, header: taglib.Id3v2FrameHeader, version: number) => {
    return taglib.ByteVector.equals(header.frameId.render(version), ts2)
        ? taglib.Id3v2TextInformationFrame.fromOffsetRawData(data, offset, header, version)
        : undefined;
};
taglib.Id3v2FrameFactory.addFrameCreator(frameCreator);

const file = taglib.File.createFromPath("path_to_file");

(this code hasn't been tested so it might require some tweaks to work as expected)

I'm gonna close this issue as completed, but feel free to reach out with other questions.

the0neWhoKnocks commented 6 months ago

@benrr101 I found the id3v2.2 tag names on https://exiftool.org/TagNames/ID3.html#v2_2. It was the top result when searching for TS2. When checking again I saw

These are the tags written by iTunes 5.0

so I guess that explains what program wrote the tags (though there's no mention of the padding. I've tested loading the file in Kid3 and it had no problem loading the tags as id3.

Not sure it's a good idea to reference a web archived page as a source of truth, especially considering the document was published in 1998. Though I also understand how hard it is to find accurate current info on tag standards.

I'll see if I can create a shim with all the 2.2 tags from the page I listed, and the example you gave for unsupported tags.

benrr101 commented 6 months ago

@the0neWhoKnocks Understood. In my experience with exiftool.org their data is mostly crowd-sourced and, for example with MPEG-4 box types, it's a bit all over the place. The link I listed was only from archive.org b/c the official id3.org website has been broken for over a year now. Nevertheless, the doc I linked is the official documentation for ID3v2.2. You're right that it's old, but it's an old, informal standard. ID3v2.3 came out in 1999 :)

Anyhow, if you'd like to correlate the missing ID3v2.2 frame identifiers from exiftool.org with the well known frame identifiers for ID3v2.3, I'd be happy to take that as a PR. Otherwise, let me know and I can add it as a task.

the0neWhoKnocks commented 6 months ago

@benrr101 So I gave it a shot with:

// Add support for obselete id3v2.2 tags (usually written by iTunes 5.0).
// Source: https://exiftool.org/TagNames/ID3.html#v2_2
const tags = [
  'COM',   // Comment
  'PIC',   // Picture (the 3 tags below are also extracted from this PIC frame)
  // 'PIC-1', // PictureFormat
  // 'PIC-2', // PictureType
  // 'PIC-3', // PictureDescription
  'TAL',   // Album
  'TCO',   // Genre (uses same lookup table as ID3v1 Genre)
  'TDA',   // Date
  'TP1',   // Artist
  'TRK',   // Track
  'TS2',   // AlbumArtistSortOrder
  'TT2',   // Title
  'TXX',   // UserDefinedText
  'TYE',   // Year
];
for (const tagName of tags) {
  const tag = ByteVector.fromString(`${tagName}\0`, StringType.Latin1);
  const id3v22FrameCreator = (data, offset, header, version) => {
    if (ByteVector.equals(header.frameId.render(version), tag)) {
      const rawFrameId = data.subarray(0, 4).toString(StringType.Latin1);
      console.log(rawFrameId);

      return Id3v2TextInformationFrame.fromOffsetRawData(data, offset, header, version);
    }
  };

  Id3v2FrameFactory.addFrameCreator(id3v22FrameCreator);
}

I see it matching the tags via console.log(rawFrameId);, but later when file.getTag(TagTypes.Id3v2); is called, I get this error:

Error: Argument out of range: index must be less than -1
    at Guards.lessThanInclusive (/home/node/app/node_modules/node-taglib-sharp/dist/utils.js:43:19)
    at ByteVector.get (/home/node/app/node_modules/node-taglib-sharp/dist/byteVector.js:693:24)
    at TextInformationFrame.parseRawData (/home/node/app/node_modules/node-taglib-sharp/dist/id3v2/frames/textInformationFrame.js:306:31)
    at get text [as text] (/home/node/app/node_modules/node-taglib-sharp/dist/id3v2/frames/textInformationFrame.js:199:14)

Tried to debug it, but there's so much obfuscation of values via frames and ByteVectors I couldn't wrap my head around it. Feels like I'm fighting the tide a bit, but it also feels like I should be able to say "treat padded tag names as version 2 tags" and it'd just utilize the functionality you've already established.