On Jun 8, 2012, at 9:13 AM, Stefan Stavrev wrote:

> I will start with the Over operation as we agreed. This is the most basic 
> signature, just to start from something. 
> As you read along, you will see me modifying it as I provide arguments why I 
> do so.
> 
> bool Over (const ImageBuf &src1, const ImageBuf &src2, ImageBuf &dst, float 
> alpha)

The alpha is part of the source image.  There's no point passing a parameter; 
if the alpha were constant over the whole image, you wouldn't need "over" at 
all, you could just use "scale" and "add".


> 
> 1. Chris already asked this before, should we have just two source images or 
> more?

I vote for just two, at least for now.  Making it take a pointer to a 
collection of images would make it different from all the other ImageBufAlgo 
functions so far, and different from most of the ones you will end up doing.  
Remember, we are trying to pick a first operation to implement that will serve 
as a canonical example for the interface and implementation that will guide all 
your others.  It's ok to have exceptions where it makes sense, but don't make 
your canonical case one of the exceptions.


> 
> At this point, there are two functions in oiiotool.cpp for each image 
> processing operation. One function handles
> input from command line, pops/pushes images on stack, etc. and then calls the 
> second function, which is the actual
> image processing operation.

Not quite.  Generally (or at least what we're striving for) is one "bare" 
function in ImageBufAlgo (often in src/libOpenImageIO/imagebufalgo.cpp and 
publicly declared in imagebufalgo.h) that operates directly on ImageBuf's, and 
a second "wrapper" that is entirely local to oiiotool.cpp that deals with 
oiiotool command lines and such.


> 2. Should we apply this operation only on one channel?

No, I've never heard of that.  The common (only?) case is to composite all 
channels in the image.


> What do we do if we have input images with different number of channels?

Fail.  With one exception: if one of the images lacks an alpha channel, we can 
treat it as if its alpha is 1.0 everywhere.  But if the two source images 
differ in the number of non-alpha channels, the over operation should fail.



> How many channels will the output image have?

(number of non-alpha channels in the input) + 1


> 3. The function should work on a specified region rather than on the whole 
> image.

Now that's an interesting question.  To support threading, there should be a 
(non-exposed) helper function that does most of the heavy lifting, entirely 
internal to imagebufalgo.cpp:

        bool over_impl (ImageBuf &dst, const ImageBuf &srcA, const ImageBuf 
&srcB,
                        int xbegin, int xend, int ybegin, int yend, int zbegin, 
int zend);

This is critical internally because the main wrapper function (the public 
"over") will want to spawn threads for large images, each of which subdivides 
the domain.  (See discussion below.)



So, some questions for anybody still reading:

Question #1: Would you like the public "over" function to always operate over 
full image size, or should it expose the "region of interest" arguments?  If 
the latter, do you like the "xbegin,xend,ybegin,..." approach, or for the sake 
of reducing the arguments passed around is it better to create a "ROI" 
structure that holds these?

Question #2: Should ImageBufAlgo functions such as over resize the destination 
image (for example, to the union of the extents of the inputs), or is it the 
caller's responsibility to already have initialized the size, channels, and 
data format of the destination image, and the IBA function merely fills in the 
pixel values for whatever range of pixels it already contains?

            
> 
> 4. Use more threads if available.

Look in src/maketx/maketx.cpp at the parallel_image template and how it's used. 

Maybe I'll clean this up and move it to ImageBufAlgo so it's easier for you to 
use.


> Final result: Over (const ImageBuf* srcs, ImageBuf &dst, float* alphas, int 
> channel, int threads)

I vote for:

        bool ImageBufAlgo::over (ImageBuf &dst, const ImageBuf &srcA, const 
ImageBuf &srcB,
                                 int threads=-1);

The default value (< 0) for threads means "use the number of threads specified 
by the attribute("threads")", which itself defaults to the number of cores 
available.


--
Larry Gritz
[email protected]


_______________________________________________
Oiio-dev mailing list
[email protected]
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org

Reply via email to