On Fri, Jul 27, 2012 at 4:24 PM, Stefan Stavrev <[email protected]> wrote:
> "Do you have any particular use cases in mind? Does other well known
> software support this use case? I'm on the fence with this one: on the
> one hand I can see that it might be useful, on the other hand it makes
> the C++ API more difficult to use, unless I'm mistaken."
>
> My goal was to give the API user maximum control, to be able to apply
> contrast per channel with different parameters. It definitely
> makes the API usage uglier. Instead of providing us with
> two floats for contrast and pivot, he has to prepare two std::vector<float>.
> But if we allow for contrast per channel, the API is already ugly, so why
> not have pivot per channel, I guess that was my reasoning. Or, we could back
> off from the idea contrast per channel and keep it simple for now by having
> one contrast and one pivot applied to all channels.
I simply ask because I want to know we're getting something useful for the
complexity. I really don't have an opinion on whether this is necessary ;-)
>From an API /usage/ point of view, I think the clean way to handle this is
four contrast functions:
// simple version
bool contrast (ImageBuf &R, const ImageBuf &A,
float contrast, float pivot,
ROI roi = ROI(), int threads = 0);
// Assumes pivot is an array with length == A.spec().nchannels
bool contrast (ImageBuf &R, const ImageBuf &A,
float contrast, float* pivot,
ROI roi = ROI(), int threads = 0);
// Assumes contrast is an array with length == A.spec().nchannels
bool contrast (ImageBuf &R, const ImageBuf &A,
float contrast*, float pivot,
ROI roi = ROI(), int threads = 0);
// Assumes both contrast and pivot are arrays with length == A.spec().nchannels
bool contrast (ImageBuf &R, const ImageBuf &A,
float contrast*, float* pivot,
ROI roi = ROI(), int threads = 0);
Behind the scenes these all probably just call through to the last version I
guess.
> On second look on all this here is what I suggest:
>
> 1. For API user:
> Let him specify contrast and pivot per channel with two std::vector<float>.
> If there is one element in the vector, it means use that value for all
> channels. If there are A.nchannels() elements then apply each value
> to its respective channels. For any other number of elements fail.
Forcing the user to use a std::vector here seems a bit unnecessary, if
somewhat safer. What if they happen to have stored their channels some other
way? What if they know their images are always three channel and want to
allocate the pivot simply on the stack? For example:
float pivot[] = {0.4, 0.5, 0.5};
contrast(R, A, 1.1, pivot);
seems more convenient and less restrictive that using std::vector.
(Side note mostly @Larry: I suppose it would be an abuse to consider using
VaryingRef<float> here ?)
> 2. For oiiotool user:
> For both contrast and pivot, if one value is given then use that for all
> channels, otherwise apply the given values to their respective channel.
> That means we would support the following 4 cases:
>
> ./oiiotool in.png --contrast:pivot=0.5 1.2 -o out.png
> ./oiiotool in.png --contrast:pivot=0.5 1.2,1.5,1.8 -o out.png
> ./oiiotool in.png --contrast:pivot=0.5,0.6,0.8 1.2 -o out.png
> ./oiiotool in.png --contrast:pivot=0.5,0.6,0.8 1.2,1.5,1.8 -o out.png
Works for me. The key point is that the extra flexibility doesn't intrude on
the simple (and presumably most common) case.
~Chris
_______________________________________________
Oiio-dev mailing list
[email protected]
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
