The histogram-computing function should live in imagebufalgo.{h,cpp} (not local
to oiiotool) and be in the ImageBufAlgo namespace, and I have a number of
suggestions for how to organize it, which I think are best explained by
proposing a function prototype:
bool
ImageBufAlgo::histogram (const ImageBuf &image, // the image
int xbegin, int xend, // range
int ybegin, int yend, // of
int zbegin, int zend, // pixels
int channel, // which channel we're
histogramming, or -1 to
// indicate luminance of the
first 3 channels
std::vector<imagesize_t> &hist, // where to store the
histogram
float min=0, float max=1, // range that the bins cover
int bins=256, // number of bins
imagesize_t *submin=NULL, // store count of vals <
min, if not NULL
imagesize_t *supermax=NULL); // store count of vals >
max, if not NULL
Analyze the pixels in the given channel of the image (special case: if channel
== -1, it means to build a histogram of the luminance of channels 0-3) over the
pixel range bounded by [xbegin,xend) X [ybegin,yend) X [zbegin,zend). The
histogram will be stored in hist[0..nbins-1] of equally-sized bins, where
hist[i] will contain the number of pixels whose values are in the range
[min+i*(max-min)/bins, min+(i+1)*(min-max)/bins). If the optional submin
and/or supermax are non-NULL, store in them the count of pixels whose values
were < min and > max, respectively. The function returns true if all is ok,
false upon error (such as nonsensical arguments).
Anybody have any further suggestions or important functionality that I've
missed?
Yeah, I'm a real stickler for API details.
On Mar 28, 2012, at 8:24 PM, Stefan Stavrev wrote:
> Ok Larry, I will do so.
>
> In the mean time, I finished the other algorithms and documented their
> usage. You can check out my last commit, I started a new branch called
> branchALL:
>
> https://github.com/StefanStavrev/oiio/commit/f2f5d7ded925f2ac5fecc3f6190939db18dc8b1e
>
> Please don't bother with the code for now, I will send small pull
> requests one at a time. But you can read the documentation and play
> with the commands. I have also provided all the test images.
>
> On 3/28/12, Larry Gritz <[email protected]> wrote:
>> What I recommend instead is a simpler function that generates a histogram
>> for only one color channel at a time:
>>
>> oiiotool inputfile --histimg <channel_num_or_name> -o outputfile
>>
>> And an extension, the channel number/name can take "luminance" to give an
>> overall luminance histogram. (I called it "histimg" to distinguish from a
>> histogram itself, which is just the mathematical table, which we may also
>> want separately some day.)
>>
>> This makes the code simpler, allows you to take a histogram of any
>> individual channel, and allows you to take histograms of channels other than
>> R, G, and/or B.
>>
>> If you make a function that magically outputs the histogram rather than
>> placing the resulting image on the "stack", that makes it hard to have a
>> more complex command that does other things to the histogram image.
>>
>>
>>
>> On Mar 28, 2012, at 11:13 AM, Stefan Stavrev wrote:
>>
>>> Larry, I looked at the ImageBuf code. I need to save three images but
>>> without the "-o" flag.
>>>
>>> ./oiiotool --histogram_rgb_components prefix dir input.tif
>>>
>>> So for one RGB image input.tif three histograms will be saved for the
>>> R, G and B components. The names of those files will be dir + prefix +
>>> "_R" for example for R. I don't want the user to specify three
>>> separate paths.
>>>
>>>
>>>
>>> On 3/28/12, Stefan Stavrev <[email protected]> wrote:
>>>> I left that to be done last, I am finishing up the rest now since they
>>>> all follow similar pattern for working with images.
>>>>
>>>> Basically all the algorithms work with images in the following ways:
>>>>
>>>> 1. one input image -> one output image
>>>> 2. more input images -> one output image
>>>> 3. one input image -> more output images
>>>>
>>>> All the algorithms but one, work like 1 and 2. Just one algorithm
>>>> works as 3, and so I left it last. For 3 I need to save more images,
>>>> that is why I asked about ImageBuf::save and did not rely on using the
>>>> stack for saving images. I will let you know what happens as soon as
>>>> possible.
>>>>
>>>> Thanks
>>>>
>>>>
>>>> On 3/28/12, Larry Gritz <[email protected]> wrote:
>>>>> That's fine, yes.
>>>>>
>>>>> Were you able to make use of that pull request I submitted related to
>>>>> the
>>>>> ImageBuf's?
>>>>>
>>>>> -- lg
>>>>>
>>>>>
>>>>> On Mar 28, 2012, at 10:14 AM, Stefan Stavrev wrote:
>>>>>
>>>>>> If not tonight, then tomorrow I should finish command line
>>>>>> functionality for all the algorithms.
>>>>>>
>>>>>> I will make a pdf for the usage of the algorithms including example
>>>>>> images of what they do. Would it be ok if I update my current branch
>>>>>> with ALL the code and give you the pdf to play with the commands?
>>>>>>
>>>>>> Then I would create a new branch and place just one algorithm in it
>>>>>> and make a pull request for that. Once that is approved, I will update
>>>>>> the code with the next algorithm and so on. Algorithm by algorithm.
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> On 3/28/12, Larry Gritz <[email protected]> wrote:
>>>>>>> On Mar 28, 2012, at 7:20 AM, Stefan Stavrev wrote:
>>>>>>>
>>>>>>>> 1. Would it be ok if I send 20 or so pull requests at once?
>>>>>>>
>>>>>>> That would be awkward. A pull request is for ALL differences between
>>>>>>> one
>>>>>>> of
>>>>>>> your branches versus our master at the point where your branch
>>>>>>> diverged.
>>>>>>> If
>>>>>>> you continue to push changes to that branch after you submit the pull
>>>>>>> request, the pull request will simply automatically update. So to
>>>>>>> make
>>>>>>> 20
>>>>>>> separate simultaneous pull requests, you need them to come from 20
>>>>>>> separate
>>>>>>> topic branches.
>>>>>>>
>>>>>>> As a practical matter, this doesn't make sense. If you have several
>>>>>>> related
>>>>>>> changes, they should all be one pull request (although it's a sign
>>>>>>> that
>>>>>>> you
>>>>>>> should have had a smaller pull request earlier, got that approved and
>>>>>>> committed, then continued from that point). If you have several
>>>>>>> unrelated
>>>>>>> changes, they should have separate pull requests, but from separate
>>>>>>> topic
>>>>>>> branches.
>>>>>>>
>>>>>>> As a more experienced developer (after you've had several commits
>>>>>>> approved),
>>>>>>> it will make sense to batch related changes up into larger pull
>>>>>>> requests.
>>>>>>> For now, I just wanted you to get the first couple small ones out of
>>>>>>> the
>>>>>>> way, it makes things easier.
>>>>>>>
>>>>>>>
>>>>>>>> 2. What if minor changes are needed for a pull request? Last time I
>>>>>>>> tried to update the code in my pull request I got some errors.
>>>>>>>
>>>>>>> It should be the case that you merely need to commit additional
>>>>>>> changes
>>>>>>> to
>>>>>>> the topic branch, then push it to your GH account. What errors
>>>>>>> exactly
>>>>>>> did
>>>>>>> you see?
>>>>>>>
>>>>>>>
>>>>>>>> 3. The license file is in dist/doc named LICENSE right? I guess I
>>>>>>>> should change the year to 2012 for this line: "Copyright 2008 Larry
>>>>>>>> Gritz and the other authors and contributors." ?
>>>>>>>
>>>>>>> No, please don't change anything you aren't directly working on. If
>>>>>>> you
>>>>>>> make a *new* file, by all means use 2012 in that comment. But there's
>>>>>>> no
>>>>>>> reason (legal or otherwise) to change the existing notices, and my it
>>>>>>> conflicts with my philosophy that commits should be minimal and
>>>>>>> orthogonal
>>>>>>> (not mixing completely unrelated changes). If we ever needed to
>>>>>>> change
>>>>>>> the
>>>>>>> license or copyright (which we don't -- it would offer no additional
>>>>>>> protection), we would do it across the board as a single commit with
>>>>>>> nothing
>>>>>>> else included.
>>>>>>>
>>>>>>> --
>>>>>>> Larry Gritz
>>>>>>> [email protected]
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Oiio-dev mailing list
>>>>>>> [email protected]
>>>>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>>>>>>>
>>>>>> _______________________________________________
>>>>>> Oiio-dev mailing list
>>>>>> [email protected]
>>>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>>>>>
>>>>> --
>>>>> Larry Gritz
>>>>> [email protected]
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Oiio-dev mailing list
>>>>> [email protected]
>>>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>>>>>
>>>>
>>> _______________________________________________
>>> Oiio-dev mailing list
>>> [email protected]
>>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>>
>> --
>> Larry Gritz
>> [email protected]
>>
>>
>> _______________________________________________
>> Oiio-dev mailing list
>> [email protected]
>> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
>>
> _______________________________________________
> Oiio-dev mailing list
> [email protected]
> http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
--
Larry Gritz
[email protected]
_______________________________________________
Oiio-dev mailing list
[email protected]
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
