Closed davidmarkclements closed 10 years ago
Hi David, when the input image is on the filesystem, libvips only reads data as it is needed further down the processing pipeline. It's not "streaming" in the Node.js sense of the word but it does minimise memory usage.
True streaming of compressed input data from a network socket would probably require modification to libvips. Summoning the vastly superior knowledge of @jcupitt, who may be able to offer help here.
btw you'd want to change the .write()
command to something else for future stream support. maybe toFile()
or writeTo()
Hi @davidmarkclements, as @lovell says, this is currently sort-of supported. When you process a large jpeg image (for example) libvips never decompresses the whole thing all at once, it just decompresses and then recompresses the area being processed as it works down the image.
If you want to work over unix pipes or network sockets, you'd need to add a new pair of load and save operations, something like vips_foreign_load_jpeg_fd( int fd, VipsImage **out, ... )
and vips_foreign_save_jpeg_fd( VipsImage *in, int fd, ... )
. The saver would be trivial, you can just reuse the file saver, but the loader would be a bit more work, you'd need to make a new libjpeg input method.
You'd then need to expose that in sharp somehow, @lovell would be the expert there.
If you're curious, the load-jpeg-from-buffer class is defined here:
https://github.com/jcupitt/libvips/blob/master/libvips/foreign/jpegload.c#L249
And it calls this set of functions to do the work:
https://github.com/jcupitt/libvips/blob/master/libvips/foreign/jpeg2vips.c#L1042
So you'd probably need to add under 500 lines of C.
:+1: for streams!
sorry for delay on this guys, look I can happily get involved in the JS side of things, in terms of creating the stream interfaces and hooking them up to the bindings (although I'd have to research that, haven't worked with bindings before). But as for actually editing the C code, it's not my area and I wouldn't want to screw it up - if someone else who's strong in C wants to team up let me know and we'll get this done - I think it would be a huge win to have true stream image processing available in JS. I really don't know of anything else right now that offers that.
OK, I'll try doing jpeg as an experiment. I've not done much with sockets, I guess I just read() and don't seek().
Would you always be reading from streams with a known type? Sniffing jpeg/png/etc. from a stream would be fiddly. I guess you'd read a mine header (something like that?) first to get the file type, or perhaps you'd be streaming from a source you controlled.
I've made an experimental branch with a quick hack to support load-jpeg-from-stream:
https://github.com/jcupitt/libvips/tree/load-from-stream
For example, you can do this:
$ cat k2.jpg | vips jpegload_fd 0 x.v
Call from C like this:
/**
* vips_jpegload_fd:
* @fd: descriptor to load from
* @out: image to write
* @...: %NULL-terminated list of optional named arguments
*
* Optional arguments:
*
* @shrink: shrink by this much on load
* @fail: fail on warnings
*
* Read a JPEG-formatted stream from @fd into a VIPS image. Exactly as
* vips_jpegload(), but read from a descriptor. This operation does not seek()
* and therefore can read from a network socket.
*
* See also: vips_jpegload().
*
* Returns: 0 on success, -1 on error.
*/
int
vips_jpegload_fd( int descriptor, VipsImage **out, ... )
We should try hooking this up to sharp somehow and see how it behaves.
I thought of a problem with this: it can only load a single image from a socket. If you want to send a series of images, you'll have to close and reopen the connection. I suppose this might be OK in some situations, but probably not over a slow network connection.
The reason is buffering. As you can see, it reads as many bytes as it can each time, up to the buffer capacity:
https://github.com/jcupitt/libvips/blob/load-from-stream/libvips/foreign/jpeg2vips.c#L1349
So it can read the end of one image and the first part of the next. When image1 is complete, we will have unused bytes buffered which we would have to be saved somehow to be used for the next image.
It's unclear how this could be done neatly. vips does not support image streams (reading many images from one source) and adding it would be hard, so that's probably out. We'd probably need to add some sort of intermediate buffer object which supported "unread", ie. allowed unused bytes to be pushed back into it.
How does node tackle this sort of issue? Do they have a socket object that supports pushback?
Thanks @jcupitt, Node.js' net.Socket object is our friend here.
I'll try to make some time over the next few days to experiment with the new vips_jpegload_fd
API and work out the most elegant way for sharp to expose it.
Using a socket would make the streams API super easier, essentially you just expose the socket directly or add methods that act as proxies to the sockets methods. But is that really the right thing to do? The normative way as I understanding is to use C bindings http://nodejs.org/api/addons.html
And then wrap a stream interface around the exposed parts of the underlying C lib. Using a socket will add unnecessary overhead IMO
Sorry on mobile didn't see @jcupitts comment what I said May or may not be relevant. Btw pushback can be done when creating streams using the Stream module but don't know if that fits what we're talking about
@jcupitt If libvips provided an API that accepted partial chunks of image data then that would fit with @davidmarkclements suggestion about managing the Streams in JavaScript.
As soon as the chunk(s) that comprise the image header data have arrived then processing can begin, but a thread(s) may have to block waiting for further chunks of image data.
This is essentially the approach taken by the Node.js wrapper around LAME, which takes advantage of the underlying library's chunk-aware methods such as lame_encode_buffer.
Sockets and Streams both involve buffering data somewhere; something will block on a lack of data :)
Shall we trial the Sockets method first as that currently feels like the quickest way to a proof-of-concept?
I was thinking about this again.
It would work if you're OK only being able to read a single image off a socket. That might be useful in some situations.
The problem is that vips (or rather the libjpeg read system) HAS to have a buffer, and therefore you can't send more then one image down a socket, since the end of the first image will eat the first part of the second.
We need to have a reusable intermediate read object to handle the buffering, and we need to be able to hand the libjpeg read system pointers into it. Therefore I think the buffer has to be a new vips object. In C, the API might look like:
int descriptor;
VipsInputStream *stream = vips_input_stream_new( descriptor );
for( i = 0; i < 10; i++ ) {
VipsImage *x;
if( vips_jpegload_stream( stream, &x, NULL ) )
return( -1 );
... do something with image x
g_object_unref( x );
}
g_object_unref( stream );
So I think we just need to get a file descriptor from the node.js stream.
After playing with Node.js Streams for the last couple of evenings I now believe the best approach is to avoid file descriptors and instead have sharp extend the stream.Transform class, buffering data in JavaScript-land first. This should then provide support for multiple images passing through the same pipe.
I've got some sample code working locally that needs tuning, documenting and benchmarking, but the early signs are that we should be able to support JPEG, PNG and WebP (but not TIFF) as both input and output streams without any modification to libvips. Hooray!
YES! That's exactly the right approach
On 19 Jun 2014, at 06:02, Lovell Fuller notifications@github.com wrote:
After playing with Node.js Streams for the last couple of evenings I now believe the best approach is to avoid file descriptors and instead have sharp extend the stream.Transform class, buffering data in JavaScript-land first. This should then provide support for multiple images passing through the same pipe.
I've got some sample code working locally that needs tuning, documenting and benchmarking, but the early signs are that we should be able to support JPEG, PNG and WebP (but not TIFF) as both input and output streams without any modification to libvips. Hooray!
— Reply to this email directly or view it on GitHub.
In terms of buffering the data in JS land, it would be great to ensure the transform stream honours the highWaterMark option (ie the capacity of the buffer) - that way a dev can control the trade off between speed and memory usage.
That's one of the problems with graphicks/image magick wrappers - lack of control over memory usage leads to crashing on under resourced servers. Whereas a stream with a configurable highWaterMark allows the dev to operate and lower resources with slower delivery time. And vice versa, if they've plenty to spare they can up the watermark number (as long as the CPU can keep up).
If there's anyway I can help let me know (peer review?)
This sounds great, but I don't really understand how it can work :-( but please prove me wrong, of course.
libjpeg's read system just has a thing for "I need more data", it does not tell you how much more data it would like. So you always read 4096 bytes (or whatever) and hand over that buffer.
After it has finished reading an image, there will be some unused bytes left in libjpeg's input buffer. These unused bytes will contain the beginning of the next image (perhaps part of a mime header?). They can't be thrown away, they somehow have to be reused. They need to be extracted from libjpeg and given to the input system of whatever image reader you need next.
So ... I still think we'll need a VipsInputStream
object or equivalent to be able to move input buffers between image readers.
libpng's input system works in the same way, it asks for more data but won't tell you how much.
libtiff needs to be able to seek on the input, so it can't read from a stream, unfortunately.
webp, again, just asks for more data.
In summary, we need to buffer image data somewhere, be that within the underlying decompression libraries themselves, in libvips or in sharp.
The memory usage problems of *magick are usually due to them holding the entire uncompressed image in memory at once. If we buffer compressed image data only, that's a big improvement over existing solutions.
What I'm suggesting is that, to start with, we have sharp manage Buffer concatenation of compressed image data as chunks arrive (I may move memory management to the C++ part of sharp if Node.js Buffer performance is poor). Then, when libvips provides support for John's suggested VipsInputStream
, we can switch to take advantage of that.
The external API of sharp will remain the same and can slowly get a bit more efficient over time, by pushing the responsibility for buffering upstream.
Here's a usage example of how elegant the Stream-based API of sharp should hopefully be, in this case resizing to 100px wide whilst maintaining the same format.
var transformer = sharp().resize(100);
inputStream.pipe(transformer).pipe(outputStream);
This unlocks the ability to create memory-efficient command line tools that pipe/transform image data from stdin to stdout.
Ah, I see. Sure, sounds great. This is using vips_image_new_from_buffer()
, I guess?
https://github.com/jcupitt/libvips/blob/master/libvips/iofuncs/image.c#L1916
I'll add a quick hack of the input stream stuff. You should be able to subclass VipsStream
and hook it up to the node.js stream system, hopefully.
edit if you've not come across it, there's a write to buffer operation too you could maybe use for the output side:
https://github.com/jcupitt/libvips/blob/master/libvips/iofuncs/image.c#L2205
@lovell I love how you've totally groked the awesomeness.
So. Something else we need to consider is handling backpressure. Since it's a transform stream it's going to need to do two things in order to be a stream that plays nice within a stream chain (and indeed within the larger OS environment).
1) if incoming data is arriving too fast (e.g. say from.. really really fast storage) the write method should return false to apply back pressure to input stream 2) if the read part of the transform stream receives false from the write stream its piping into (e.g. if the destination streams write method returns false) then it needs to pause until the destination stream is ready - you tell if a write stream is ready again by listening for a drain event
@davidmarkclements I think your 1 and 2 should happen automatically. They do on Linux at least.
VipsInput
only does a read()
when it needs more data, so the OS will apply backpressure for us when the amount of stuff queued up hits the high water mark.@jcupitt But if the stream is being piped to another stream (say the other stream was slow http connection for instance), the other stream may need to apply back pressure to sharp stream, at a JavaScript level this should ideally be catered to otherwise streams its piping to will be overwhelmed and data will begin to queue up in memory which is of course contrary to the point of streams.
As for the writing side.. we're talking about a source node stream writing to the sharp node stream, e.g. extending @lovells example a little
var fs = require('fs');
var http = require('http');
var inputStream = fs.createReadStream('/some/image.jpg');
var transformer = sharp().resize(100);
http.createServer(function (req, res) {
var outputStream = res;
inputStream.pipe(transformer).pipe(outputStream);
});
So say we the read stream from disk is coming in too fast, at a JS level we need to apply back pressure to let the inputStream know it needs to pause and wait for a drain event.
I don't see how the OS can suspend the stream in this case, because the sharp library isn't reading the file here, nor is really operating at an OS level - the OS doesn't have control or knowledge of the streams in Node. That has to be managed at a JS level.
Further the input stream doesn't even need to be a file, it could be this scenario for instance:
var fs = require('fs');
var http = require('http');
var inputStream = http.get('http://some.image.com/image.png');
var transformer = sharp().resize(100);
http.createServer(function (req, res) {
var outputStream = res;
inputStream.pipe(transformer).pipe(outputStream);
});
Each stream implementation in Node has to manage and handle backpressure, if they expect the OS to they'll quickly run into memory issues
Given initially we'll be placing input data into a memory buffer, I think we can rely on the OS for read-side back-pressure.
For write-side, sharp will have to manage any back-pressure applied to it by the OS. I've got a couple of ideas here, mostly involving reducing the output buffer data into exponentially smaller chunks.
The rather handy brake module can use to emulate back-pressure to test this.
Ah, I thought these were network sockets and local pipes.
You're right if you want it to work for JS software streams you'd need a bit more logic. This could go in the VipsStream
subclass I guess (as you maybe said).
@jcupitt yeah so piping and streams and what not are completely emulated abstractions in node
@lovell okay I'm still a little unsure on the OS management side, by read-side do you mean "the source stream" e.g.
inputStream // "read-side"
.pipe(transform)
.pipe(outputStream) // "write-side"
When the transform stream (e.g. the sharp stream) is getting too much from the inputStream, the OS may well pause the transform stream and at an OS level apply back pressure to a socket or a file or what not - but in this scenario the inputStream isn't a socket or file - it's a JS abstraction that may or may not have a socket/file underlying it. Which the transform stream has no knowledge of (or it shouldn't have anyway).
So when the at the OS level or a C level sharp realises it's reading data too quickly, at the point it should notify inputStream at the JS level so the abstraction that Node streams are built upon can reach down into its inners and say - "hold on a sec".
oh also, that brake module looks awesome. another great little module by @substack
The stream branch now runs this test program:
https://gist.github.com/jcupitt/9cd5662838cb749f706c
There's a VipsStream
object and you can reuse it to read several images. It seems to work OK.
Next I'll split VipsStream
to input and output objects and add a save jpeg to stream operation. I'll do PNG as well so we can be sure we can attach the streams to more than one image IO library. It should be possible to sniff the stream as well to automatically pick a loader.
The stream class looks like this:
https://github.com/jcupitt/libvips/blob/load-from-stream/libvips/include/vips/stream.h
You should be able to subclass that and make a vips_stream_new_from_node()
or whatever and have a stream object that hooks vips directly up to a node.js stream.
An initial commit to add support for Streams is available on the experimental streams
branch.
npm install lovell/sharp#streams
It buffers data from the input stream in JavaScript, uses the existing libvips' *load_buffer
methods, then attempts to push the entire output image buffer to the output stream in one hit.
I need to add backpressure handling/backoff to the output (write-side), fully document this and create far more extensive tests, but it's a start.
Regarding input (read-side) backpressure, I don't expect network or disk IO to be able to provide data faster than we can allocate memory buffers for, but would be happy to add it later if/when someone runs into that problem.
Hey, we're getting closer. Looks like we'll meet in the middle quite soon.
I just got jpeg load and save to streams going. The libvips stream branch now runs this program:
https://gist.github.com/jcupitt/ff21f5b8d1dd6ba54c6f
If you run it, you see:
$ ./a.out x x2
vips_stream_input_new_from_filename: x
vips_stream_input_build: 0x15ad210
vips_stream_output_new_from_filename: x2
vips_stream_output_build: 0x15afc30
vips_stream_read: read 4096 bytes
vips_stream_attach:
vips_stream_attach:
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_output_write: written 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_read: read 4096 bytes
vips_stream_output_write: written 4096 bytes
vips_stream_output_write: written 4096 bytes
... etc
You can see data flowing in and out as it runs down the image. I've put in png load/save as well, I'll do something to sniff filetype from the stream next.
Filetype sniffing is done too, I've updated the gist.
The inner loop now looks like this:
while( !vips_stream_input_eof( in_stream ) ) {
VipsImage *x;
VipsImage *y;
if( !(x = vips_image_new_from_stream( in_stream, "", NULL )) ||
vips_invert( x, &y, NULL ) ||
vips_image_write_to_stream( y, ".jpg", out_stream, NULL ) )
vips_error_exit( NULL );
g_object_unref( x );
g_object_unref( y );
}
So the input stream has a series of PNGs and JPGs, they are inverted and written to the output as .jpg. It all seems to work fine.
It think it's ready to join up with sharp
. I'll make a quick example showing how to subclass the input and output stream objects to link them up to something other than a pipe.
... and here's a gist showing how to subclass vips streams, no idea if the commenting is enough to make it understandable :-(
Final comment on this!
There are some downsides to the true read-from-stream thing. @lovell, your buffer-to-ram approach is probably better overall.
May image formats, like tiff, simply don't stream. These would need to be buffered to RAM or disc and then processed from there. vips can process tiffs without having to hold the whole uncompressed image in memory at once, so there's still an advantage there.
Things like jpeg shrink-on-load, vital for good performance, need to be set at open time. But unfortunately you don't know the image size then to calculate the shrink-on-load factor.
It's important for PNG as well: interlaced PNG files can't be streamed, you need to hold the entire uncompressed image in memory before you can start. If you want to handle this horrible case nicely, you need to know the image is interlaced before you open it.
I guess you'd need to have a pre-processing stage to extract image metadata from the stream, yuk.
So, this would be why no one's tackled this yet.
It'd still be awesome to be able to stream the things that can be streamed.
I've updated the streams branch to include documentation, functional and benchmark tests.
I've removed attempts at back-pressure handling as Node.js seems to already provide this when you push a single Buffer to an output Stream (I'm using the same approach as the built-in zlib library).
@davidmarkclements are you able to review and/or test my approach?
npm install lovell/sharp#streams
Yep be happy to, flying from Aus this weekend so it might not be till next week
@lovell So I'm just taking a cursory look over readme example at the moment, is there anything stopping you from making the return value a stream (as well as a promise) e.g.
sharp('input.png').rotate(180).resize(300).sharpen().quality(90).webp().pipe(outputStream)
I also think the example could be made clearer by defining the outputStream
Stream to file:
var toFile = require('fs').createReadStream('output.webp');
sharp('input.png').rotate(180).resize(300).sharpen().quality(90).webp().pipe(toFile)
Stream across http:
var http = require('http');
var path = require('path');
http.createServer(function (req, res) {
sharp('input.png').rotate(180).resize(300).sharpen().quality(90).webp().pipe(res);
}).listen(8080);
Not meaning to bikeshed here, it just occurs to me that the canonical example of most modules I've worked with tend to be
1) module creates an stream 2) it pipes to something else
There's nothing wrong with the transform approach I'm just thinking for those getting their head around streams etc. in relation to sharp it adds extra conceptual complexity.
So I realise that creating the fs readStream on behalf of the user when they do sharp('input.png') may complicate things internally - another option might be
sharp({streamFrom: 'input.png'}).resize(200, 300).pipe(outputStream);
Certainly not as nice but could be an okay half way point.
OR
sharp().stream('input.png').resize(200, 300).pipe(outputStream);
But perhaps "stream" is too unclear. Anyway, that's just API stuff, I'll check the approach when I have bit more time (going through airports at the mo so probably not focused enough for internals anyway)
(Back on this feature after a short break.)
Thanks @davidmarkclements for the feedback. Anything that makes this module easier for people to use is a good thing. How does the following sound:
The sharp
constructor can accept a Readable Stream from which it will stream the input data. A new pipe
method will accept a Writable Stream, where the output format will match input format. The existing jpeg
, png
and webp
methods will be modified to also accept a Writable Stream.
This means it will be possible to mix and match files, Buffer objects and Stream objects for the input and output, e.g. Stream in, file out; Buffer in, Stream out; etc.
Unlike with the stream.Transform version, at no point will a Stream subclass be returned. The API will be Stream-like with the provision of a pipe
method, but the need to support files, Buffers, Promises and Streams with a fluent API is too much for my tiny brain to also work out support for pure OO constructs right now.
Hey @lovell
I think any further stream integration is good, but these would be my observations
1) A "stream-like" route is trouble, if its stream-like people will try to use it as streams... and become confused when certaint hings don't work. If I saw docs saying stream-like but not streams it would make me think twice ("I dont know whether this is going to work for my case" vs "ah its a stream, great that'll work").
2) accepting writable streams into webp methods etc is okay I suppose, but it still misses the point of streams a bit
3) The main point would be, is it compatible with others streams via pipes (as thats the main emphasis on streams, much like *nix platforms)
If I get a chance I'll take a look and see if theres an easy way to make the API return streams at the same point it returns promises?
OR if keeping to th fluent api is too painful, create a parallel api thats for streams, e.g. sharp.stream'
I think I've worked out how to correctly inherit from stream.Duplex with some minor API breakage.
The jpeg
, png
and webp
methods need to become simple property setters and therefore can no longer trigger the processing pipeline without an explicit call to toFile
, toBuffer
or pipe
.
Currently writing lots of tests.
Hey @lovell sounds awesome, just to confirm, in case there's anything else that needs hooking up, the .on('data', fn)
and/or stream.Duplex.prototype.read
would also trigger the processing pipeline right?
Yes, the pipeline will be triggered lazily only when _read
is called internally to notify us that the consumer is ready. Sorry for any confusion.
Commit https://github.com/lovell/sharp/commit/6c9f3de622ee8fcb4cc6ae8ee2feeb3cc5849715 in the streams2 branch implements the native stream.Duplex object.
The tests from line 272 onwards provide good examples of the various methods of input and output.
Comments welcome!
I've updated the streams2
branch to not (yet) break the backwards compatibility of accepting a callback with the png
, jpeg
and webp
methods. They will still, for now, trigger the pipeline but also log a deprecation warning that this feature will be removed in the next major version.
Any final thoughts from @sebpiq, @jonathanong and @davidmarkclements before this hits master?
The streams2
branch is now merged to master
and will be included in the forthcoming v0.6.0 release.
Thanks to everyone involved for all the help getting this far.
hey many that's awesome - sorry for lack of final thoughts
Closing this feature request - thank you everyone for all the suggestions and feedback - I look forward to hearing all about your use of Streams.
Hello, I've spent a few days implementing experimental support for true streaming:
https://github.com/libvips/libvips/pull/1443
It seems to work. Any testing or feedback very welcome.
Does the whole buffer have to load into run time memory or is it possible with the underlying C library to implementing a true streaming api, so that a file can be streamed from disk, through sharp stream to network socket or file read stream or whatever else.
ref: http://nodejs.org/api/stream.html
Neither graphicks magik, nor image magik wrappers are capable of streams - theres one module (gm I think) that has a stream api, but its faked somewhat, the module still has to load the full image into memory regardless. So a true streaming interface for sharp would be a massive usp