search

sandreas / tone

tone is a cross platform audio tagger and metadata editor to dump and modify metadata for a wide variety of formats, including mp3, m4b, flac and more. It has no dependencies and can be downloaded as single binary for Windows, macOS, Linux and other common platforms.
https://pilabor.com
Apache License 2.0
417 stars 17 forks source link

stdout maxBuffer length exceeded error with v0.1.8 #69

Closed JakobTischler closed 2 days ago

JakobTischler commented 5 days ago

Any tone tag ... command I've tried with v0.1.8 ends up in an stdout maxBuffer length exceeded error. Using v0.1.7 does not produce this error with the same commands. I'm using tone-0.1.8-win-x64.zip on Windows 10.

Example output:

RangeError: stdout maxBuffer length exceeded
    at Socket.onChildStdout (node:child_process:490:14)
    at Socket.emit (node:events:519:28)
    at Socket.emit (node:domain:488:12)
    at addChunk (node:internal/streams/readable:559:12)
    at readableAddChunkPushByteMode (node:internal/streams/readable:510:3)
    at Socket.Readable.push (node:internal/streams/readable:390:5)
    at Pipe.onStreamRead (node:internal/stream_base_commons:191:23)
    at Pipe.callbackTrampoline (node:internal/async_hooks:130:17) {
  code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER',
  cmd: `tone tag "Y:/Audio/Podcasts_TEMP/Comedy Bang Bang/2024/Comedy Bang Bang - 879 - Side Duck (2024-08-26) [Haley Joel Osment, Jon Gabrus, Vic Michaelis].mp3" --auto-import=covers --meta-artist "Scott Aukerman, Haley Joel Osment, Jon Gabrus, Vic Michaelis" --meta-sort-artist "Aukerman, Scott" --meta-album-artist "Scott Aukerman" --meta-sort-album-artist "Aukerman, Scott" --meta-album "Comedy Bang Bang" --meta-composer "Haley Joel Osment, Jon Gabrus, Vic Michaelis" --meta-cover-file "Y:\\Audio\\Podcasts\\Comedy Bang Bang\\cover.jpeg" --meta-genre "Podcast//Comedy" --meta-disc-number "1" --meta-title "Side Duck" --meta-additional-field "year=2024-08-26" --meta-additional-field "date=2024-08-26" --meta-additional-field "releaseDate=2024-08-26" --meta-publishing-date "2024-08-26" --meta-additional-field "language=eng" --meta-additional-field "podcast=1" --meta-track-number "879" --meta-movement "879" --meta-comment "Academy Award nominated actor Haley Joel Osment joins Scott to talk about his new movie Blink Twice, where he would live for 180 days out of the year, and the 25th anniversary of The Sixth Sense. Then, intern Gino Lombardo returns to talk about his recent run-in with the law 
in Nassau County for wearing a mask. Plus, Pokémon trainer Ember Chuckit stops by to talk about a pilot program that Professor Oak is running. Get tickets for the Comedy Bang! Bang! Into Your Mouth Tour 2024 over at <a href='https://cbbworld.com/tour' rel='noopener noreferrer' target='_blank'><u>https://CBBWorld.com/tour</u></a>Check out The Action Boyz Live on Saturday, September 7th at The Dynasty Typewriter in Los Angeles. For tickets go to <a href='https://live.actionboyz.biz/' rel='noopener noreferrer' target='_blank'><u>live.actionboyz.biz</u></a>" --meta-additional-field "unsyncedLyrics=eng||Academy Award nominated actor Haley Joel Osment joins Scott to talk about his new movie Blink Twice, where he would live for 180 days out of the year, and the 25th anniversary of The Sixth Sense. Then, intern Gino Lombardo returns to talk about his recent run-in with the law in Nassau County for wearing a mask. Plus, Pokémon trainer Ember Chuckit stops by to talk about a pilot program that Professor Oak is running. Get tickets for the Comedy Bang! Bang! Into Your Mouth Tour 2024 over at <u>https://CBBWorld.com/tour</u>Check out The Action Boyz Live on Saturday, September 7th at The Dynasty Typewriter in Los Angeles. For tickets go to <u>live.actionboyz.biz</u>" --meta-sort-album "Comedy Bang Bang" --meta-movement-name "Comedy Bang Bang"`
}

Many thanks for your help.

sandreas commented 5 days ago

Mmh since I released 0.1.8 today there may be a problem in a library I have to investigate. On my Linux it works as expected... Maybe I should un-release the 0.1.8 for now until this is clear. Thanks for reporting this so quick.

Probably this has to do something with the AnsiConsole-Width problem (#68)

sandreas commented 5 days ago

@JakobTischler So it seems to me that you are using tone via JavaScript / NodeJS and posted the according error message, that probably does not have to do anything with tone's error system?

I'm a bit confused, because tone also has a JavaScript engine and I'm not sure where this error might come from...

Here is my guess:

Would that be possible?

JakobTischler commented 4 days ago

You're possibly correct. I'm indeed using this in a NodeJS environment.

I async call child_process→exec inside a promise:

import { exec } from 'child_process';

export default function commandPromise(command: string) {
    return new Promise((resolve, reject) => {
        exec(command, (error, stdout, stderr) => {
            if (error) {
                return reject(error);
                // throw error;
            }
            if (stdout) {
                console.log(stdout);
                resolve(stdout);
            }
            if (stderr) {
                return reject(stderr);
                // throw stderr;
            }
        });
    });
}

If I understand you correctly, it would be child_process.exec's buffer problem. I've found this StackOverflow answer to a similar problem. The solution would be to simply increase the maxBuffer with the exec call.

I'll try it out later tonight and will let you know.

sandreas commented 4 days ago

I'll try it out later tonight and will let you know.

Ok, since I unreleased tone 0.1.8 because of this problem, you might have to use your existing version.

However, while the fix could have worked, it seems that there are edge cases like yours, that have to be handled more carefully and require more thinking.

I probably go for the solution to use Console.WriteLine on non spectre-console output. Thanks for the quick feedback.

JakobTischler commented 4 days ago

So that didn't work. I tried different buffer sizes, from 500 kB to 200 MB. All ended in stdout maxBuffer length exceeded.

sandreas commented 4 days ago

So that didn't work. I tried different buffer sizes, from 500 kB to 200 MB. All ended in stdout maxBuffer length exceeded.

Thanks for the feedback. I'll try to fix this the right way and then release 0.1.8 again.

sandreas commented 3 days ago

@JakobTischler Could you give me the complete command you are executing?

Here is the problem: I found no decent way to fix #68 (which is a really nasty bug that MUST be fixed) without the lines that break your code.

What I could do is provide either an environment variable TONE_OUTPUT_REDIRECT=0 (or an extra parameter (like --no-output-redirect) that you could set to fix your use case. I really don't want to do this, but every suggested solution (e.g. use Console.WriteLine instead of AnsiConsole.WriteLine) did not work in my tests.

If anyone can provide a solution that works without this pretty annoying workaround, I'd be happy to listen...

JakobTischler commented 3 days ago

I have one command in the error log in the opening post, but here it is for reference:

tone tag "Y:/Audio/Podcasts_TEMP/Comedy Bang Bang/2024/Comedy Bang Bang - 879 - Side Duck (2024-08-26) [Haley Joel Osment, Jon Gabrus, Vic Michaelis].mp3" 
    --auto-import=covers 
    --meta-artist "Scott Aukerman, Haley Joel Osment, Jon Gabrus, Vic Michaelis" 
    --meta-sort-artist "Aukerman, Scott" 
    --meta-album-artist "Scott Aukerman" 
    --meta-sort-album-artist "Aukerman, Scott" 
    --meta-album "Comedy Bang Bang" 
    --meta-composer "Haley Joel Osment, Jon Gabrus, Vic Michaelis" 
    --meta-cover-file "Y:\\Audio\\Podcasts\\Comedy Bang Bang\\cover.jpeg" 
    --meta-genre "Podcast//Comedy" 
    --meta-disc-number "1" 
    --meta-title "Side Duck" 
    --meta-additional-field "year=2024-08-26" 
    --meta-additional-field "date=2024-08-26" 
    --meta-additional-field "releaseDate=2024-08-26" 
    --meta-publishing-date "2024-08-26" 
    --meta-additional-field "language=eng" 
    --meta-additional-field "podcast=1" 
    --meta-track-number "879" 
    --meta-movement "879" 
    --meta-comment "Academy Award nominated actor Haley Joel Osment joins Scott to talk about his new movie Blink Twice, where he would live for 180 days out of the year, and the 25th anniversary of The Sixth Sense. Then, intern Gino Lombardo returns to talk about his recent run-in with the law 
in Nassau County for wearing a mask. Plus, Pokémon trainer Ember Chuckit stops by to talk about a pilot program that Professor Oak is running. Get tickets for the Comedy Bang! Bang! Into Your Mouth Tour 2024 over at <a href='https://cbbworld.com/tour' rel='noopener noreferrer' target='_blank'><u>https://CBBWorld.com/tour</u></a>Check out The Action Boyz Live on Saturday, September 7th at The Dynasty Typewriter in Los Angeles. For tickets go to <a href='https://live.actionboyz.biz/' rel='noopener noreferrer' target='_blank'><u>live.actionboyz.biz</u></a>" 
    --meta-additional-field "unsyncedLyrics=eng||Academy Award nominated actor Haley Joel Osment joins Scott to talk about his new movie Blink Twice, where he would live for 180 days out of the year, and the 25th anniversary of The Sixth Sense. Then, intern Gino Lombardo returns to talk about his recent run-in with the law in Nassau County for wearing a mask. Plus, Pokémon trainer Ember Chuckit stops by to talk about a pilot program that Professor Oak is running. Get tickets for the Comedy Bang! Bang! Into Your Mouth Tour 2024 over at <u>https://CBBWorld.com/tour</u>Check out The Action Boyz Live on Saturday, September 7th at The Dynasty Typewriter in Los Angeles. For tickets go to <u>live.actionboyz.biz</u>" 
    --meta-sort-album "Comedy Bang Bang" 
    --meta-movement-name "Comedy Bang Bang"

I've done some tests just now concerning command length being used, and had some interesting results. As you can see in the command, the comment tag is rather long. Additionally, the same content is added to the unsyncedlyrics tag, with unsyncedLyrics=eng||....

If I remove the comment tag, it still fails. If I remove the unsyncedLyrics tag, though, it goes through. I'm not sure if I'm right on the cusp of the max buffer length, or if there's another problem with unsyncedLyrics.

EDIT: Sometimes. It sometimes goes through, so still seems to be a size problem.

sandreas commented 3 days ago

If I remove the comment tag, it still fails. If I remove the unsyncedLyrics tag, though, it goes through. I'm not sure if I'm right on the cusp of the max buffer length, or if there's another problem with unsyncedLyrics.

You could try to generating a temporary tone.json file instead (tone tag --meta-tone-json-file /tmp/use-these-tags.json my-file.mp3). This would prevent having to long commands exceeding the buffer.

I'd suggest the following solution:

In case it really is the new Console.WriteLine handling that causes your problems, you could just set the environment variable in your code as a workaround (untested, just a draft):

import { exec } from 'child_process';

export default function commandPromise(command: string) {

    return new Promise((resolve, reject) => {
        exec(command, {env: Object.assign({}, process.env, {"TONE_OUTPUT_REDIRECT": "0"})}, (error, stdout, stderr) => {
            if (error) {
                return reject(error);
                // throw error;
            }
            if (stdout) {
                console.log(stdout);
                resolve(stdout);
            }
            if (stderr) {
                return reject(stderr);
                // throw stderr;
            }
        });
    });
}

What do you think?

sandreas commented 2 days ago

Fixed in 0.1.8 - see https://github.com/sandreas/tone?tab=readme-ov-file#dump-tags - section "redirecting output".

JakobTischler commented 1 day ago

@sandreas Thanks for the workaround. Just got around to testing, and as you predicted it works like a charm.

Thanks for your help, and - as always - for your work in general.

sandreas commented 1 day ago

Thanks for your help, and - as always - for your work in general.

You're very welcome :-)