Add buffer support

[kpanic]

hello @talklittle this is just a work in progress, however I wanted to have a feedback from you about this approach before going further.

See issue https://github.com/route/mogrify/issues/56

[talklittle]

This seems like a good start. I can see that Mogrify.create/2 is now writing the stdout contents to a binary, good.

I can't tell what Mogrify.save/2 is doing with the buffer, maybe it's unfinished? Does the mogrify command-line program support stdout?

We definitely want to make use of the System.cmd/3 option :into which would allow us to put the results into any kind of Collectable, not just an in-memory binary, which would help avoid out-of-memory issues for large images. https://hexdocs.pm/elixir/System.html#cmd/3-options

Thank you for your work on this!

[kpanic]

hello! Yes save/2 is just unfinished. I'll checkout :into. Thanks for the pointer!

In your opinion, should all the file functions in Mogrify.* be able to return a buffer? Or, does it make sense only for create/2?

Thanks!

[talklittle]

The functions that write to a file should be able to create a buffer. So create/2 and save/2

[kpanic]

Hey @talklittle, could you please (if you know) provide an example regarding mogrify outputting to stdout and System.cmd/3 returning an IO.Stream? I tried:

iex(14) {stream, 0} = System.cmd("convert", ["pango:\"<span foreground=\"yellow\">hello world</span>\"", "jpg:-"], into: IO.binstream(:stdio, :line))
# binary output here
iex(13)> stream                                                                                                                                       
%IO.Stream{device: :standard_io, line_or_bytes: :line, raw: true}

Any hints?

Thank you in advance! :)

[talklittle]

I don't know if mogrify command line supports pipes and stdout. That may be a major roadblock to this issue. We can release the feature for convert only, so create/2 and not save/2, if that is the case.

[kpanic]

cool, good point regarding the mogrify command. I checked the man page, and it says that is possible, however I was not able to make it work. Probably my fault.

ok, I'll work on it (convert) by returning the full binary, since I did not understood yet how the into: IO.binstream(:stdio, :line) incantation works. I could adapt it later on

[talklittle]

For the :into option we could simply add an identical :into option in our create/2 function, defaulting to "", which we pass directly to the System.cmd call. Whoever is using our library can optionally decide to pass a different Collectable when calling create/2.

The rest remains the same (returning the first element of the tuple returned by System.cmd). Except the variable name should change from "binary_image" to "image_collectable"

[kpanic]

Good idea @talklittle ! I very like it, will change accordingly in the next free time that I will have! ;)

[kpanic]

Please check the last 3 commits, in particular https://github.com/route/mogrify/pull/62/commits/63ca685770335a2ae332d900c3408a046621cfce

I avoided to have into: "" as default since it's already a default :) If into: IO.stream/2 will be provided then it will be used Thanks!

[talklittle]

Yes, perfect. And good job on the test.

The only remaining thing is it would be good to make the 3 create/2 implementations return something similar. So maybe add :buffer as a field on Mogrify.Image struct, instead of returning it directly. I can do that part if it's not interesting to you.

Otherwise, are you done with the PR?

[kpanic]

Yes, I am done. Feel free to take over the :buffer field in Mogrify.Image struct. It's evening here and I had a couple of glasses of :wine_glass: -- chilling :wink: Otherwise I could continue tomorrow's evening! As you wish :)

[talklittle]

Okay I've merged for now but will not put out a release until the cleanup, which I can look into.

Thank you for the contribution!

[kpanic]

Sure, thank you for your assistance!

[aselder]

@kpanic @talklittle This is awesome, and is something that I will use a ton. (Currently writing a sparkline PNG generator). Any idea on a release time frame?

[talklittle]

Just released mogrify 0.7.0 with buffer support in create/2 but not save/2