Closed cliri21 closed 5 years ago
I've looked at the source code and it seems multer-s3
expects a writeable stream.
simple-thumbnail
's API currently doesn't support piping in input. I just got back from work and I haven't had supper yet, but I'll look into changing the API to suit your needs.
I'm thinking of adding the following to the API:
genThumbnail('250x?') // Returns a duplex stream
Thoughts?
I've looked at the source code and it seems
multer-s3
expects a writeable stream.
simple-thumbnail
's API currently doesn't support piping in input. I just got back from work and I haven't had supper yet, but I'll look into changing the API to suit your needs.I'm thinking of adding the following to the API:
genThumbnail('250x?') // Returns a duplex stream
Thoughts?
That would be awesome!
Note to self: ffmpeg -y -i pipe:0 -vframes 1 -ss 00:00:00 -vf scale=50:-1 -f singlejpeg pipe:1
Alright, a new version was published. To get a duplex stream, make the following call to genThumbnail
:
genThumbnail(null, null, '250x?') // Returns a duplex stream
You should now be able to get simple-thumbnail
to work with multer-s3
. Here's an example (I haven't tested on S3, hopefully it'll work though?)
var upload = multer({
storage: multerS3({
s3: s3,
bucket: 'some-bucket',
shouldTransform: function (req, file, cb) {
cb(null, /^image/i.test(file.mimetype))
},
transforms: [{
id: 'thumbnail',
key: function (req, file, cb) {
cb(null, 'image-thumbnail.jpg')
},
transform: function (req, file, cb) {
cb(null, genThumbnail(null, null, '100x100'))
}
}]
})
})
Thank you for creating this issue; I learned a lot about custom streams :smile:
Thanks for the quick update, greatly appreciate it! I managed to get the duplex stream working with mutler-s3. However, I noticed that the _read is finished before the _write, and for me it threw a write EPIPE error that was killing my app. I managed to create a hacky solution in the meantime, but you probably know best how to handle this issue, I am assuming this is because it is inputting the whole video file, but only outputting the thumbnail which is much smaller in size?
Can you please share the exact error? I'm little bit rusty with Node.js (for personal and professional projects, I've moved to Golang), so having an error stack trace would be very helpful.
Also, what was your "hacky solution"? Perhaps it'll help me pinpoint the problem? When I wrote the test, I couldn't get the .on('finish', cb)
handler to work (perhaps this issue is related?), see: https://github.com/ScottyFillups/simple-thumbnail/blob/master/test/test.js#L184
Also, posting your node version would be appreciated. Mine is v11.15.0
The exact error I got and nothing more was:
{ Error: write EPIPE at WriteWrap.afterWrite [as oncomplete] (net.js:868:14) errno: 'EPIPE', code: 'EPIPE', syscall: 'write' }
My 'hacky' solution was just ignoring the error that the aws-sdk was catching 😅, and everything worked as expected. So not really a solution I suppose.
I believe mutler-s3 is closing the stream after _read is finished but before _write is finished, which is what triggers this specfic error. But keep in mind, I don't know much about streams and I am assuming this is what's causing the issue.
I noticed what's specfically being called multiple times is this section in the _write:
this.ffmpeg.on('close', () => { this.end() })
Multiple 'close's are being called after _read is complete, the bigger the file, the more times it is called.
Oh shoot, yeah that's definitely a bug (this.ffmpeg.on('close', () => { this.end() })
). I don't know what I was thinking at the time; I'm surprised my test didn't complain.
this.end()
should only be called once. I'm going to research the error message in case there's another issue with my implementation. That code can be moved in the constructor; I'm unsure whether that will solve all the issues on your end though.
Yep, moved that close code to the constructor, and it is now only called once, so that fixed that. However, still getting that error though. Im on node v8.12.0 btw.
Might just be an error specific to my implementation though. So maybe there isn't anything wrong with your package specifically. At least we managed to catch the close bug though, haha.
Yeah, based on comments here, it seems like this error is linked to the "pipee": https://github.com/nodejs/node/issues/947
I'll make a minor release to fix the bug though. Some time I hope to figure out why I can't get on('finish', cb)
to work. Thanks helping me catch the bug, greatly appreciated :smile:
I plan to play around with the code a bit more before closing this issue, but there's the possibility that the code on your end is complaining since AWS closes its write stream before consuming all the data?
Sounds good.
Yeah don't stress yourself out too much about it, its probably something on my end. I will figure it out though and get back to you. Thanks again!
Yeah, please let me know if you figure out why on('finish', cb)
doesn't work: https://github.com/ScottyFillups/simple-thumbnail/blob/master/test/test.js#L184
The commented out code should work, but for some reason it doesn't. If I had to guess, that's the reason you're experiencing issues.
@cliri21
The fix I made will likely fix your issue; I realized that the duplex stream I created did not emit ffmpeg
's underlying events, which explains a lot of the funkiness we were seeing.
@ScottyFillups Works flawlessly now! Thank you so much!!!
Can I see an example using a read stream and null as output. I'm trying to get it to work with https://github.com/gmenih341/multer-s3 in the transform function of that package. Would be greatly appreciated!