Create an image manipulation module

[daveaglick]

It should be able to convert images as well as possibly create images from scratch (using SVG? System.Drawing?). Would be good for things like image-based text headers, etc.

[dodyg]

I just discovered this project today. Kudos.

This http://imageprocessor.org/ is a very good library to process images on the fly. Let me figure out how Wyam works first. I would very much to contribute on this module.

[daveaglick]

Awesome - I'd be happy for any help you can give. Yep, @JimBobSquarePants is a good guy so I'm sure he'd be happy to answer questions if needed, though the actual image library use should be fairly simple for this module. Anyway, just give a shout if you need guidance or help with the module (if you decide to tackle it), or with Wyam in general.

[JimBobSquarePants]

Hey guys,

I would imagine most of the work you would need to do would be a wrapper round the ImageFactory class from ImageProcessor. If you list what functionality you need to use I'm sure I can help design the process. :smile: For SVG we'd probably have to do something combining it with Cairo.

[daveaglick]

See, what'd I say - good guy :) The SVG thing was just initial spitballing, in retrospect I'm not sure how valuable SVG support would be or even what use case it would solve.

In any case, the original issue was (intentionally) vague on requirements. If he decides to take it up, I'll leave it up to @dodyg to figure out what he thinks might be valuable initial functionality and we can take it from there.

Thanks for following-up, @JimBobSquarePants!

[JimBobSquarePants]

Hehehe No worries. OSS is what it's all about :smile: @dodyg I'm sure you can build something easily enough using ImageProcessor but if you get stuck just tag me and I'll help.

[dodyg]

Awesome.

I will start with some basic scenarios:

• Resize image by a specific width
• Resize image by a specific height
• Resize image by a specific width, height and specify cropping behavior (top, bottom, etc) or using Constraint
• Apply filter

Ideally the generated images will have predictable flags suffixes (e.g. myimage-w34-h54.jpg) to make it easier for javascript/css manipulation.

[dodyg]

I am building prototyping two modules to accomplish this. One is called ProcessFiles which takes sub modules. It behaves similarly with ReadFiles module but it will read file as stream and pass it to the child modules. The other is called ImageProcessor

So it will look roughly like

ProcessFiles("*", ImageProcessor()).Where(x => Path.GetExtension(x) == ".gif" || Path.GetExtension(x) == ".png" || Path.GetExtension(x) == ".jpg")

[daveaglick]

That seems like a good approach for the way things currently work, but it makes me wonder about the current architecture. I presume the need for the ProcessFiles module is because IDocuments are designed to pass string content through the pipeline, which wouldn't support images and other binary content? While using a module to read files into a stream and then pass it (assuming via metadata?) to submodules should work, it's not terribly flexible. For example, what if I wanted to source an image from a web url instead of a file on the file system? Ideally, I should be able to swap out the part where I get the binary content and still use the part that processes it. That's sort of the whole concept of Wyam and why it's different and flexible.

I hadn't really gotten to thinking too much about manipulation beyond text. This does make me wonder if there needs to be some fundamental way of dealing with binary content. Perhaps an additional object to 'IDocument' that goes through the pipeline and holds streams or blobs instead of strings. Or maybe change 'IDocument' itself to hold binary or string data. Or maybe a whole different kind of pipeline (and module) for streams and/or binary data.

Let me give this some thought this weekend - it's brought up some very good questions. In the meantime, you should be able to proceed. Whatever happens to the architecture (if anything), the underlying image manipulation code should still be valid.

[daveaglick]

So after some initial thought, my gut reaction is that IDocument should use a Stream instead of a string for content. Then modules can act accordingly depending on what they do (such as text-based modules passing-through binary content). This would be similar to what OWIN does.

If that were the case, you wouldn't have a need for ProcessFiles and could just stick ImageProcessor right in the main pipeline after ReadFiles (which would now just read whatever into a stream).

Anyway, that's what I'm thinking right now - but I reserve the right to change my mind :) It'll take a little bit of work to adapt the existing modules, caching, etc. to use streams but nothing too difficult. Look for these changes in the dev branch sometime next week (I'm away for the weekend).

[dodyg]

Sounds good.

[JimBobSquarePants]

:100: on streams. You'll save yourself a whole world of pain there.

[dodyg]

Right now ProcessFiles converts file byte[] to Base64 and puts it in IDocument.Content and then ImageProcessor module has to convert it back to byte[]. It's not exactly the fastest way to get the job done :laughing:

[dodyg]

First iteration:

Pipelines.Add("ImageProcessing",
    ProcessFiles("*", ImageProcessor(new ImageInstruction(100,100), new ImageInstruction(60, null), new ImageInstruction(null, 600))).Where(x => new []{".png", ".jpg", ".jpeg", "gif"}.Contains(Path.GetExtension(x)))
);

https://github.com/Wyamio/Wyam/compare/develop...dodyg:develop

The purpose of ImageInstruction is to allow multiple processing of the images in one go. The problem is that it forces people to use new which looks out of place. The other alternative is simply to have people defines ImageProcessor multiple times or define fluent method SetInstruction.

[daveaglick]

Looking good! Two suggestions:

• Can we put the new modules in a seperate library (maybe Wyam.Modules.Images or something)? I'm trying to keep the dependencies in Wyam.Core to an absolute minimum. That'll help the embedding story and also help when I go to port to Core/DNX/etc. and cross-platform (since not all dependencies are going to support all platforms). Then just add a project reference to the new library in the Wyam application project and it'll pull in the new modules automatically for the command line app.
• I like your idea of using a fluent interface rather than instantiating a config class. That'll match with the convention used by other modules, and I think it'll make for a nice terse syntax once I get the stream bits working:

Pipelines.Add("ImageProcessing",
    ReadFiles("*"),
    ImageProcessor()
        .ImageInstruction(100, 100)
        .ImageInstruction(60, null)
        .ImageInstruction(null, 600)
        .Where(x => new []{".png", ".jpg", ".jpeg", "gif"}.Contains(Path.GetExtension(x))),
    WriteFiles()
);

[dodyg]

Yeah more modules should be separate from core. I will iterate more on the fluent interface because there will be more options available for the image processor.

[dodyg]

2nd Iterations

Pipelines.Add("ImageProcessing",
    ProcessFiles("*").Where(x => new []{".png", ".jpg", ".jpeg", "gif"}.Contains(Path.GetExtension(x))),
    ImageProcessor()
        .Resize(100,100).ApplyFilter(ImageFilter.Sepia)
        .And.Resize(60, null)
        .And.Resize(0, 600)
);

https://github.com/Wyamio/Wyam/compare/develop...dodyg:develop

[dodyg]

I am going to create Wyam.Modules.Extras as a catch all place to hold non-essential modules (ImageProcessor, etc). Or should it be Wyam.Modules.Contrib?

Or should all modules have their own project? The downside of this is the proliferation of projects consisted of just small number of files/codes. The good thing off course you can package individual module on nuget.

[dodyg]

• Add Brighten and Darken fluent methods. These two methods correspond to ImageFactory.Brightness value. It takes values from 0 to 100.
• Mirror all the values of ImageProcessor's IMatrixFilterin ImageFilter

Pipelines.Add("ImageProcessing",
    ProcessFiles("*").Where(x => new []{".png", ".jpg", ".jpeg", "gif"}.Contains(Path.GetExtension(x))),
    ImageProcessor().Resize(100,100).ApplyFilter(ImageFilter.GreyScale).ApplyFilter(ImageFilter.Comic)
    .And.Resize(60, null).Brighten(30)
    .And.Resize(0, 600).Darken(88)
);

https://github.com/Wyamio/Wyam/compare/develop...dodyg:develop

[daveaglick]

This is really shaping up - good work.

Personally, I've been aiming for library separation based on dependencies. That gets back to eventually packaging different versions of Wyam for the alternate .NET runtimes coming out. The ability of a given module to run on a given platform is going to be primarily dependent on dependencies and what they can support. By making lots of libraries, each delineated by the use of a specific dependent library, we can create a table that says "On CoreCLR you can use these modules, on XYZ you can use these other modules". I don't mind a proliferation of projects and NuGets - I think that just re-enforces the idea that the tool is modular.

TL;DR: I would create a library Wyam.Modules.ImageProcessor and put in all the modules that use the ImageProcessor library. Then when/if you make other modules that use other libraries, I would put them in a different project.

[dodyg]

OK clear. This explanation should probably go up at http://wyam.io/knowledgebase/.

[dodyg]

• Create Wyam.Modules.ImageProcessor and move ProcessFiles and ImageProcessor there
• Change ApplyFilter to ApplyFilters

Pipelines.Add("ImageProcessing",
    ProcessFiles("*").Where(x => new []{".png", ".jpg", ".jpeg", "gif"}.Contains(Path.GetExtension(x))),
    ImageProcessor().Resize(100,100).ApplyFilters(ImageFilter.GreyScale, ImageFilter.Comic)
    .And.Resize(60, null).Brighten(30)
    .And.Resize(0, 600).Darken(88)
);

https://github.com/Wyamio/Wyam/compare/develop...dodyg:develop

[daveaglick]

@JimBobSquarePants You mentioned that using streams would "save a whole world of pain" - could you elaborate? I'm familiar with problems caused by storing large strings, binary arrays, etc. due to running out of memory, but am also having a hard time convincing myself that it'll be a real problem in this case and is worth the loss of convenience of operating against a raw string or byte[]. I'm curious what sort of real-world issues you've seen when not streaming data. If you have a moment, I've been arguing this point with myself over here: https://github.com/Wyamio/Wyam/issues/42

[JimBobSquarePants]

@daveaglick Of course. What I mean by that is extensibility, encoding and performance. By using a stream for all you'll be much less likely to have to refactor your core API in the future. I've made that mistake in the past with ImageProcessor and now have to maintain legacy code that I'm dying to remove. With streams you can be environmentally independent in that you are not tied to the file system of the current machine you are on.

With a stream you can also can take advantage of all the sensible defaults in the NET encoders/decoders to ensure that you use the correct formats and can handle big/small endian environments. Someone, somewhere will lodge an issue caused by incorrect encoding at some point I guarantee it.

Lastly, performance. If you start making copies of the byte array with images for example you will definitely run out of address space on 32bit machines. For image processing you need 1GB contiguous address space so even images as small as 4000x3000 can cause issues. This is possible with other operations also and since you will be allowing multiple different implementations of you interface you definitely do not one killing the ability to run another.

You've also touched things like asynchronous operations in that thread. If you can use those methods, I would. Debugging usually isn't that much of a pain and the performance gains when used well can be excellent.

@dodyg Whilst I'm commenting I'll share with you some work I've been doing in ImageProcessor.Web that you might be able to copy. The images, I output from the factory are as optimized as the current .NET framework will allow but their not really web ready in my opinion. That link points to some code I use to further shrink images using the best-in-class libraries for optimizing images.

[dodyg]

@JimBobSquarePants thanks for the tip. I will take a look at this once I am done with the low hanging fruit part of the ImageProcessor module.

[dodyg]

ImageProcessor now integrates with the new pipeline

Pipelines.Add("ImageProcessing",
    ReadFiles("*").Where(x => new []{".png", ".jpg", ".jpeg", "gif"}.Contains(Path.GetExtension(x))),
    ImageProcessor().Resize(100,100).ApplyFilters(ImageFilter.GreyScale, ImageFilter.Comic)
    .And.Resize(60, null).Brighten(30)
    .And.Resize(0, 600).Darken(88)
    .And.Constrain(100,100)
);

https://github.com/Wyamio/Wyam/compare/develop...dodyg:develop

The next step is to pass the processed stream to the next pipeline and get out of the business of writing the stream to disk.

[dodyg]

Pipelines.Add("ImageProcessing",
    ReadFiles("*").Where(x => new []{".png", ".jpg", ".jpeg", "gif"}.Contains(Path.GetExtension(x))),
    ImageProcessor().Resize(100,100).ApplyFilters(ImageFilter.GreyScale, ImageFilter.Comic)
    .And.Resize(60, null).Brighten(30)
    .And.Resize(0, 600).Darken(88)
    .And.Constrain(100,100),
    WriteFiles()

Now the processed image is passed to the next pipeline.

[daveaglick]

:+1: Now we're talking - this is looking really good!

[dodyg]

@daveaglick I need feedback on a new fluent method ForEachDocument.

Pipelines.Add("ImageProcessing",
    ReadFiles("*").Where(x => new []{".png", ".jpg", ".jpeg", "gif"}.Contains(Path.GetExtension(x))),
    ImageProcessor().ForEachDocument(WriteFiles()).Resize(100,100).ApplyFilters(ImageFilter.GreyScale, ImageFilter.Comic)
    .And.Resize(60, null).Brighten(30)
    .And.Resize(0, 600).Darken(88)
    .And.Constrain(100,100)
);

I am trying to enable the scenario where image is saved to disk one by one after processing instead of waiting for the whole result to be ready for next pipeline.

My question is whether the name is any good. I look at the prior art ContentModule.ForEachDocument which has similar idea but different implementation.

[daveaglick]

Ah, interesting. ContentModule.ForEachDocument() is a little confusing and is actually more about how input documents are processed than how they're handled afterwards. Modules get an IEnumerable<IDocument> as input. The .ForEachDocument() flag comes into play when you want to run those input documents through child modules. In that case, you can either run them through the sequence of child modules one at a time, starting fresh each time or run them through all at once. This can be an important distinction depending on how the child modules treat their inputs.

What you're trying to accomplish is a little bit different and has some important implications (which I'll get to below) but for now, let's see how we could do it with a flag similar to .ForEachDocument(). The constraint here is that for this approach to work, the WriteFiles module has to be a "child" of ImageProcessor instead of being next in the pipeline. With a ForEachDocument(...) fluent method like the one you have above, the psuedo-code would probably look something like:

private IModule[] _forEachModules;

// ...

public ForEachDocument(params IModule[] forEachModules)
{
    _forEachModules = forEachModules;
}

// ...

public IEnumerable<IDocument> Execute(IReadOnlyList<IDocument> inputs, IExecutionContext context)
{
    foreach(IDocument document in inputs)
    {
        Stream documentStream = document.Stream;
        // Do some fancy image processing
        IDocument result = document.Clone(documentStream);
        if(_forEachModules == null)
        {
            yield return result;
        }
        else
        {
            foreach(IDocument childResult in context.Execute(_forEachModules, new []{ result }))
            {
                yield return childResult;
            }
        }
    }
}

Now, while this should do what you want, I'm not sure it's the best long-term solution to this kind of problem. The module architecture is currently designed to have one module finish it's work before the next module is run. This helps keep everything linear and makes configuration debugging easier. Unless a child module contributes to the transformation being performed (for example, child modules of Replace might get the replacement text) it's probably better as the next module in the pipeline. The danger of using too many child modules and adding child module support to every module is that it becomes unclear when to use what.

Which I guess comes back to a question: why do the image processing results need to be output as processing completes rather than processing them all and passing the aggregate set of processed images to the next module (be it WriteFiles or something else)? The answer to this will help frame how (or if) we make architecture changes to address this sort of case.

Hopefully that all made sense...

[dodyg]

I am thinking of memory pressure. In general source images are in much larger size than text. 400KB of image is a smallish size for an image. It is a large size of text.

Image processing always work from large source image (perhaps 2 - 4 mb source files) and gets processed and cut down to many pieces of different sizes.

So a single source image can easily be processed into 6 - 10 different sizes (thumbnails, retina, etc). The number of images to be stored in memory can grow much bigger fast.

So if you are waiting for all these processed images in memory before writing them to disk, we are going to encounter OoM issue very quickly.

[daveaglick]

Yeah, that makes sense - if processing an entire gallery or folder of images, that could add up very quickly. This problem will probably manifest in other modules that deal with lots of data too. There probably should be a systemic way to deal with this rather than rely on each module to work around it.

First thought: Right now a pipeline does something like a breadth-first traversal. However, some pipelines (like those that deal with images) would be better served with a depth-first traversal. That is, as each module yields a new result document, it should be passed to the next module before requesting another document from the current one.

Under the hood this will probably require changes to tracing (so the indentation doesn't get messed up), execution context result document caching, and pipeline control flow. From a configuration standpoint this should be presented as easy as setting a flag when creating the pipeline. The modules shouldn't even need to know what's going on (though it should be documented that lazy enumeration within a module is better). I don't think the changes will be too hard - I'll take a closer look tomorrow.

[daveaglick]

Okay, I implemented a ForEach module that can do what you need - take a look at the last comment in #47. That was the easiest way to get this kind of behavior without compromising the current design concept. I think it'll work pretty well.

[dodyg]

Great. I am almost done with this module.

[daveaglick]

Implemented in #52 - thanks again!