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

Example using read stream from multer-s3-transform #47

Closed
cliri21 opened this issue May 10, 2019 · 16 comments
Closed

Example using read stream from multer-s3-transform #47

cliri21 opened this issue May 10, 2019 · 16 comments

Comments

@cliri21
Copy link

cliri21 commented May 10, 2019

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!

@philipjscott
Copy link
Owner

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?

@cliri21
Copy link
Author

cliri21 commented May 11, 2019

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!

@philipjscott
Copy link
Owner

Note to self: ffmpeg -y -i pipe:0 -vframes 1 -ss 00:00:00 -vf scale=50:-1 -f singlejpeg pipe:1

@philipjscott
Copy link
Owner

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 😄

@cliri21
Copy link
Author

cliri21 commented May 12, 2019

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?

@philipjscott
Copy link
Owner

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

@philipjscott philipjscott reopened this May 12, 2019
@philipjscott
Copy link
Owner

Also, posting your node version would be appreciated. Mine is v11.15.0

@cliri21
Copy link
Author

cliri21 commented May 12, 2019

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.

@philipjscott
Copy link
Owner

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.

@cliri21
Copy link
Author

cliri21 commented May 12, 2019

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.

@cliri21
Copy link
Author

cliri21 commented May 12, 2019

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.

@philipjscott
Copy link
Owner

Yeah, based on comments here, it seems like this error is linked to the "pipee": nodejs/node#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 😄

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?

@cliri21
Copy link
Author

cliri21 commented May 12, 2019

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!

@philipjscott
Copy link
Owner

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.

@philipjscott
Copy link
Owner

@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.

@cliri21
Copy link
Author

cliri21 commented May 12, 2019

@ScottyFillups Works flawlessly now! Thank you so much!!!

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