Incorrect timestamps on Syncronized lyrics about node-id3 HOT 16 CLOSED

The value is not the same because timeStamp must be an integer value. If you replace your values with an integer value, the timeStamp will be correct, this will also fix the issue with the missing line.

from node-id3.

@Sandakan You can update to version 0.2.6 and your problem should be fixed

https://www.npmjs.com/package/node-id3/v/0.2.6

@Zazama Thanks for the update and the quick fix. 😁😁🎉🎉

from node-id3.

@Sandakan thanks for reporting the bug! Thanks @Zazama for the quick fix!

from node-id3.

Is 12.5 the second? Because you have to pass the millisecond (e.g. 12500 in this case).

This is also partially a bug: https://github.com/Zazama/node-id3/blob/0.2/src/ID3FrameBuilder.js#L17
The call to Number.isInteger(value) is false but no buffer is appended, which makes the frame be out-of-spec. The Ls in line 2-4 (and line 5 alltogether) are missing because they are consumed by the timeStamp (4294855680 => 0xFFFE4C00 => L).

from node-id3.

I created #147 to improve the documentation and prevent silent failure in appendNumber.

from node-id3.

Is 12.5 the second? Because you have to pass the millisecond (e.g. 12500 in this case).

This is also partially a bug: https://github.com/Zazama/node-id3/blob/0.2/src/ID3FrameBuilder.js#L17 The call to Number.isInteger(value) is false but no buffer is appended, which makes the frame be out-of-spec. The Ls in line 2-4 (and line 5 alltogether) are missing because they are consumed by the timeStamp (4294855680 => 0xFFFE4C00 => L).

Yes, this is exactly what I found out.

from node-id3.

The value is not the same because timeStamp must be an integer value. If you replace your values with an integer value, the timeStamp will be correct, this will also fix the issue with the missing line.

It worked this time. 😁😁

from node-id3.

There's another problem.
Whenever I update the lyrics, it will add a new lyrics object to syncronizedLyrics array instead of updating the previous one.
Is there a way to clear the array before writing it again with the new lyrics or should I use this approach and just read the last item in the array?

from node-id3.

Is 12.5 the second? Because you have to pass the millisecond (e.g. 12500 in this case).

Thanks @Zazama for pointing this out, @Sandakan did you see this point?

from node-id3.

There's another problem.
Whenever I update the lyrics, it will add a new lyrics object to syncronizedLyrics array instead of updating the previous one.
Is there a way to clear the array before writing it again with the new lyrics or should I use this approach and just read the last item in the array?

This is working as designed.

You should notice in the README documentation that the synchronisedLyrics takes an array of definition. From the id3 specs, there can be more than one. Internally, this frame is defined as multiple here.

When doing an update, the API should not modify the other frames. Therefore adds a new one. We understand that a user may want to completely replace all the lyrics. This is a known issue with the current design of the API and is the same for all the frames which can accepts multiple values.

The workaround for now is to not use the update function and instead read all the tags, replace the synchronisedLyrics one and rewrite them. Notice that this may remove some tags from your file that are not supported by this library. But I think we should support almost all the known official tags.

This will be improved in future versions.

from node-id3.

What if I keep adding new lyrics objects to the array whenever I update the song and read only the last element of the array? This should work, right?
I hope that there are no limitations to the number of lyrics objects that can be put into this array.

from node-id3.

As I said I recommended not to use the update function when manipulating lyrics, otherwise you will keep adding lyrics frames to the file, this is most likely not what you want to do. Do a read, update the result and write everything back.

Just to be clear there are two arrays and there is no limitation in neither of them:

{
    synchronisedLyrics: [{ // The top level one, for example one for each language
        language: "ENG",
        timeStampFormat: 2,
        contentType: 1,
        shortText: "this is short text",
        synchronisedText: [ // The synchronised text one
          {
            text: "Lyrics line 1",
            timeStamp: 0,
          },
        ],
    }
}

from node-id3.

I tested it with some other lyrics.

{
		contentType: TagConstants.SynchronisedLyrics.ContentType.LYRICS,
		language: 'ENG',
		timeStampFormat: TagConstants.TimeStampFormat.MILLISECONDS,
		shortText: 'this is short text',
		synchronisedText: [
			{ text: 'Never thought I would love again', timeStamp: 3610 },
			{ text: 'Here I am, lost in Love Me Land', timeStamp: 12380 },
			{ text: '♪', timeStamp: 20340 },
			{ text: 'How dare you have lips like that?', timeStamp: 29400 },
			{ text: "How dare you gon' kiss like that?", timeStamp: 34150 },
		],
	};

And this is the output I got.

{
		language: 'ENG',
		timeStampFormat: 2,
		contentType: 1,
		shortText: 'this is short text',
		synchronisedText: [
			{ text: 'Never thought I would love again', timeStamp: 3610 },
			{ text: 'Here I am, lost in Love Me Land', timeStamp: 12380 },
			{ text: '♪', timeStamp: 5207295 },
			{ text: '﹈ow dare you have lips like that?', timeStamp: 29400 },
			{ text: "How dare you gon' kiss like that?", timeStamp: 34150 },
		],
	},

In the third sychronisedText line, the timestamp is incorrect and the line after that has an unknown letter instead of the letter "H".

from node-id3.

#148 @pbricout I tried to fix it, any thoughts before merging?

When merged I can backport to branch 0.2 to release a fix

from node-id3.

@Zazama having a look at #148 .

from node-id3.

@Sandakan
You can update to version 0.2.6 and your problem should be fixed

https://www.npmjs.com/package/node-id3/v/0.2.6

from node-id3.