Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finish event not fired ? #5

Closed
PJanko opened this issue Mar 3, 2018 · 9 comments
Closed

Finish event not fired ? #5

PJanko opened this issue Mar 3, 2018 · 9 comments

Comments

@PJanko
Copy link

PJanko commented Mar 3, 2018

Hi,

I'm using Vinyl File to represents a file stream in my application. I use clones to pipe the same content to multiple ffmpeg commands (using stream-transcoder or fluent-ffmpeg (same behaviour happening)).

It works great for the first original Vinyl file, but not for the clones.
I've isolated the error in the example below, if I run it multiple times, sometimes I got the 3 finish events, sometimes I just have the one for the original stream. But the files are really copied in the example.

const cloneable = require('cloneable-readable');
const path = require('path');
const fs = require('fs');

let stream = cloneable(fs.createReadStream(path.join(__dirname, 'sample.wav')));

function pipe(stream, num) {

    stream.on('end', () => {
        console.log(`Reaching end of stream : ${num}`);
    });

    stream.pipe(fs.createWriteStream(path.join(__dirname, `sample-${num}.wav`))).on('finish', () => {
        console.log(`Stop writing to file for stream : ${num}`);
    });

}

setImmediate(pipe.bind(null, stream, 1)); // Pipe in another event loop tick <-- this one finished only, it's the original cloneable.
pipe(stream.clone(), 0);    // Pipe in the same event loop tick
setTimeout(pipe.bind(null, stream.clone(), 2), 1000);   // Pipe a long time after

setTimeout(() => { }, 2000);    // here to maintain a ref in the event loop longer.

Here is an example of what I can see during my tests :

screen shot 2018-03-03 at 18 48 21

Environment

I'm using Node v6.10.3 on mac.
I use a sample wav file of 450kb.

Full example

I think it's not an ffmpeg issue, but for your interest, here is an example of code using ffmpeg which is not working :

const path = require('path');
const fs = require('fs');
const cloneable = require('cloneable-readable');
const ffmpeg = require('fluent-ffmpeg');

let stream = cloneable(fs.createReadStream(path.join(__dirname, 'sample.wav')));

function pipe(stream, num) {

    stream.on('end', () => {
        console.log(`End writing the stream : ${num}`);
    });

    ffmpeg(stream)
        .audioFrequency(8000)
        .format('s16le')
        .output(path.join(__dirname, 'copy', `sample-${num}.wav`))
        .on('end', () => {
            console.log(`finished for the stream: ${num}`);
        })
        .run();

}

setImmediate(pipe.bind(null, stream, 1)); // Pipe in another event loop tick <-- it's the original cloneable.
pipe(stream.clone(), 0);    // Pipe in the same event loop tick
setTimeout(pipe.bind(null, stream.clone(), 2), 1000);   // Pipe a long time after

setTimeout(() => { }, 2000);    // here to maintain a ref in the event loop longer.

If you run the process above, it will never exit. Events are still attached in the event loop ? Are they lost somewhere ?

Thank you for the time and the response,
Pierre.

@mcollina
Copy link
Owner

mcollina commented Mar 5, 2018

Can you please verify if #6 solves your issues?

@mcollina
Copy link
Owner

mcollina commented Mar 7, 2018

I cannot reproduce this problem directly.

@PJanko
Copy link
Author

PJanko commented Mar 7, 2018

It's working great now. I've detected all the 'finish' events, and fluent-ffmpeg handles it as expected.

Thank you for the rapid fix !

@PJanko
Copy link
Author

PJanko commented Mar 11, 2018

Hi,

It seems that now there is some data loss when a clone is made. From the same example above, all the 'finish' events appears, the original clone is exactly the same as the original file, but the clones are not the same size.

A test might be this one :

const path = require('path');
const fs = require('fs');
const cloneable = require('cloneable-readable');
const ffmpeg = require('fluent-ffmpeg');

let stream = cloneable(fs.createReadStream(path.join(__dirname, 'sample.wav')));

function pipe(stream, num) {

    stream.on('end', () => {
        console.log(`End writing the stream : ${num}`);
    });

    stream.pipe(fs.createWriteStream(path.join(__dirname, 'copy', `sample-${num}.wav`))).on('finish', () => {
            console.log(`finished for the stream: ${num}`);
        });

}

setImmediate(pipe.bind(null, stream, 1)); // Pipe in another event loop tick <-- it's the original cloneable.
pipe(stream.clone(), 0);    // Pipe in the same event loop tick
setTimeout(pipe.bind(null, stream.clone(), 2), 1000);   // Pipe a long time after

All the finished event are handle properly, but the resulting files are not the same size.

Thanks,
Pierre.

@mcollina
Copy link
Owner

was this bug present in the previous release? Have you checked the hashes of those files?

@mcollina
Copy link
Owner

@PJanko can you please verify if #9 solves the problem?

@PJanko
Copy link
Author

PJanko commented Mar 12, 2018

It's okay for the file size, but another problem is remaining on my side. If I use PassThrough streams instead of the clones I get the expected results, but I think it's not a good practice, am I right ? Sorry for the lack of precisions, I'll try to find the reason as soon as possible.

@PJanko
Copy link
Author

PJanko commented Mar 12, 2018

To be more precise, I use websockets to receive audio binary stream. I want to multiplex the received stream into multiple processes and apply audio conversion using ffmpeg. There is an error raised somewhere in the stack. It doesn't seems to be this library.

I use socket.io for the ws support, but i'll give a try to the websocket-stream module instead. It might be a better choice for binary data streaming. I'll stay in touch.

@mcollina
Copy link
Owner

I will release the changes asap then.

Feel free to open a new issue for the new bug, it’s very unclear to me what you are doing and I need a way to reproduce to help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants