ffalt / jamp3

id3 & mp3 library
MIT License
9 stars 2 forks source link

Quiet crash when writing APIC frame #328

Closed dogwatch closed 3 years ago

dogwatch commented 3 years ago

Hello! ;D

const {ID3v2, ID3V24TagBuilder} = require('jamp3');

(async () => {
    const id3v2 = new ID3v2();
    const tb = new ID3V24TagBuilder();

    tb.picture(3, '', 'image/jpeg', Buffer.alloc(16247, 0));    // Falls if size >= 16247
    //tb.picture(3, '', 'image/jpeg', Buffer.alloc(16246, 0));  // Works if <= 16246

    await id3v2.write('example.mp3', tb.buildTag(), 3, 0, {defaultEncoding: 'ucs2'});

    console.log('OK.');

})().catch(console.log);

node 15.3.0 and 15.4.0 jamp 0.4.2

dogwatch commented 3 years ago

Broke in 0.4.0

ffalt commented 3 years ago

Hi! ^^

Thanks for reporting!

My findings so far: It's working fine in Node 14.x. Something may have changed in Node 15.x. Also, executing your sample code works if the 'example.mp3' does not exists. After I runned it twice, trying to pipe data https://github.com/ffalt/jamp3/blob/e5f79dbd4ba43a9fe65df40036af4216e0260cab/src/lib/common/stream-writer-file.ts#L44 node is not triggering any 'error' or 'end' callback, so the process quits silently. Don't know why, yet.

ffalt commented 3 years ago

It turns out, in Node 15 a backpressured stream (as occured in your example) can not be piped to. I fixed it and released 0.4.3.
Thanks again for reporting!

dogwatch commented 3 years ago

Thank you very much! Everything is working. ;)

Also, executing your sample code works if the 'example.mp3' does not exists.

I don’t have enough imagination, to understand, why write a tag to a non-existent file. Can make an option for this?

ffalt commented 3 years ago

Glad, the update is working. That bug could have occured in many file writing scenarios.

Writing tags to their own & new files scan be useful. Maybe not for everybody, but frankly I see no need for an option.