Add ability to print image from image data in memory

[shermp]

This PR adds the ability for FBInk to print an image given a pointer to an image in memory.

What I've done is extract the STB image loading out of the main printing function. The main printing function has been renamed to fbink_print_image_data(). The STB loading function keeps the original fbink_print_image() function signature, to maintain backwards compatibility.

fbink_print_image() now loads the image from a file, and then calls fbink_print_image_data(). Alternatively, users can call fbink_print_image_data() directly if they do not wish to load an image from file.

fbink_get_image_channels() has been added to assist callers of fbink_print_image_data(), so that they know what image format should be supplied.

Tested on my H2O, with RGBA at least. File loading still appears to work fine, so hopefully I haven't broken anything!

[NiLuJe]

I'll take a deeper look at it tomorrow, so just a quick & random set of comments:

That's roughly how I'd imagined doing it (not that there are billions of way to go about it anyway ;)), although I hadn't thought to bother with the req_n bit, possibly thinking: feed me rgb(a) or gtfo ;).

The fact that data is freed by the function and not the caller might feel non-intuitive for fbink_print_image_data() callers though, but OTOH I can't think of a nice way to make the distinction...

EDIT: Without changing the signature, that is. Or possibly a bit of code duplication.

[shermp]

As far as the RGBA matter, I was trying to keep things as close as possible to how they already were, FBInk currently expects stbi_load and friends to convert images to the correct format.

I'm embarrassed to say I didn't think of where the data was freed (oops, I've been in go land too much the past several days :p ). However, the simple solution would probably be to pull the freeing out of fbink_print_image_data(), and let the caller (and fbink_print_image()) free the data. I'll see how easy this is, and update the PR accordingly

[NiLuJe]

I'll double-check tomorrow with a working brain, but I don't think the caller actually needs to care about the channel count: stbi accepts anything, and req_n depends on the fb & ignore_alpha, so the caller doesn't really have to know how all of this works (nor care about it).

I think that stbi figures everything on its own, you just feed it a buffer length, and everything just works ;p. It'll return the width, height, and image channel count all on its own.

Well, at least that was my experience when using load_from_memory for stdin handling ;).

[NiLuJe]

Unless you just wanted to get at req_n for science? ;p

[NiLuJe]

Yep, that works ;).

[shermp]

From what I can see in the code, req_n is used a lot. n is the value that is basically unused. From reading the stbi docs, req_n determines the overall size in bytes of the data buffer, so it's pretty important. From what I understand, the value stored in the pointer n is purely informational to say "this is the format the original image was in"

[NiLuJe]

AFAICT, it still expects that data to be encoded in one of the supported formats, and not raw scanlines, so it doesn't need to know about the channel count to decode the data properly: the headers of the image format will answer that.

We indeed do care about req_n more than n, but it's interesting for stbi itself, to know how it'll have to output the decoded image data, and to FBInk, because that in turn dictates how we'll process it.

But the caller doesn't need to know/care about any of that: it just fills a buffer with the full data of an image encoded in a supported format, but the details of that can stay opaque, the only thing stbi needs to know is how much data there is (i.e., the buffer's length).

[NiLuJe]

Because if we just have raw scanlines as input, we don't need stbi at all: just tell FBInk what you're feeding it (i.e., req_n), and one of many branches in that giant function will do the right thing :D.

(Since raw scanlines are what we get from stbi, and req_n is what tells us how to handle them).

[shermp]

Note that I didn't go through the whole FBInk image print monstrosity myself, are you saying that it's happy to accept RGBA scanline despite whatever format is required for the framebuffer?

Because yes, the goal is to feed raw scanlines to FBInk. My usage is to draw directly to an image.RGBA in Go, and then feed the raw pixels directly into FBInk. No need for intermediary conversion steps then :)

I'll let you sleep on it, and you can give me your thought's when you have a chance to have a proper look.

I'm surprised you're awake TBH! It's late afternoon here on the other side of the world.

[NiLuJe]

Yeah, that's what I finally got (I was initially under the mistaken impression you wanted to use stbi_load_from_memory(), which might explain the crossed wires :D).

So, in that case, yeah: just tell FBInk the width, height, and how many components there are, and things will mostly be okay.

With a somewhat large caveat right now: <= 8 bpp fbs will only know how to deal with n 1 or 2; and >= 16bpp fbs with n 3 or 4.

Making that work in every combination possible is doable, but slightly annoying ;).

(And now I get why you wanted to know req_n :D).

[shermp]

All good.

In many ways stbi_load_from_memory() would certainly be easier, but hey, the less encode/decode steps, the better IMO.

[NiLuJe]

Definitely ;).

So that leaves one question: do we require G8/G8A input on 4/8bpp & RGB/RGBA input on 16/24/32, whith a simple function outputting what the device expects (and then FBInk won't even need to know the details of n: just if there's an alpha channel or not), or do we go the extra mile and handle any possible input? (It'll make that giant mess of a function even messier, and it's mildly annoying, because we have to handle switching to/from grayscale ourselves. It's also mostly boring and easy to stupidly get wrong code ;p).

[shermp]

Probably the former would be my thought. Are there even many (any?) modern ereaders that are 4/8bpp anymore? Note, I am a biased Kobo owner, so I don't know what those Kindle heathens are up to :D

[NiLuJe]

Given that all Kindles fall under the 4/8 bpp case, and all Kobo fall under the 16/32 one, I can live with the first one (it's also way less boring work for me, so, that's a plus ;D).

[NiLuJe]

Then we're agreed ;).

So to recap:

• Keep fbink_get_image_channels, but simplify it to basically return two different values: grayscale or not. I'd maybe rename it to something slightly more explicit?

• fbink_print_image_data still needs to know the exact input channel count, and error out if g8 is passed to a Kobo (!grayscale), or rgb to a Kindle (grayscale).

(Which gets slightly tricky because we can't use n or req_n for that: since in the stbi case, n can be 1 but req_n 3, or vice-versa).

[shermp]

Sounds good.

Maybe a simple boolean function fbink_greyscale_image_required() or similar?

[NiLuJe]

Yup, that's what jumped to mind after having written that ;).

[NiLuJe]

And on that note, I'm off to bed, because I'm starting to have trouble with basic reading comprehension ;D.

[NiLuJe]

Note to self: instead of trying to handle all n/req_n combos in the rendering stage, we could simply fudge the data buffer if a mismatch is detected (i.e., do the grayscale tweak in a new buffer, and render that one).

[NiLuJe]

@shermp: Now that, despite my addled brain, we've agreed on what to do, do you want me to finish polishing this up, or do you have some unpublished code?

(Sorry about the delay, I've been a bit busy these last few weeks ;)).

[shermp]

I haven't done anything with this since, so you can polish it up if you wish.

I've been working on my other project in the meantime, and had a week where IRL issues got in the way of doing anything.

[NiLuJe]

No worries, just wanted to make sure no work/time would be wasted ;).

[NiLuJe]

Okay, working on this in the raw-scanlines branch :).

I started from scratch mainly to better wrap my head around it, because as we've seen earlier, I can be a bit thick ;p.
But this generally follows the same ideas & path as this.

I managed not to break file/stdin handling, I'll check if I messed up raw handling after lunch, once I've implemented a C testcase ;).

[NiLuJe]

Implemented in #18 ;).

Thanks again for all your input, it's very much appreciated!