Additional Feedback


Jeremy Selan <jeremy...@...>
 

<Jeremy: this has been edited to remove 'personal' comments>

Commenter:

Answering your questions...

* Is this project conceptually useful to your organization?

Yes, we would love a cross application colour library that gives
consistent results on all applications with minimal user effort and
developer pain.

* Would you use it? (or recommend using it?)

Probably, we'd like to understand it in a bit more detail, and we
would need to sit down and discuss how/when/where we used it.

* Are there design choices we've made that limit its usefulness?

Straight up, for Nuke to user the CPU path, we'd need to pass in
separate pointers for R, G and B data, as they are allocated
separately and do not live at fixed strides from each other.

* Are there commercial tools you'd like us to work on support for?

Truelight

* Do you know of something better (open source), which we're not
aware of?

No

################################################################################

I've looked over the lightly commented header file, the explaining
text and the comments from others so far. While I understand that this
is the API at the moment and will no doubt change, I'm not sure of the
exact relationship between objects/entities within it. While I get the
basic ideas, as in what a colour space is, what colour timing is, I'm
not sure of some of the details, eg: what is the difference between a
Look and a FilmLook, are colour spaces simply to deal with things like
log/lin or do they extend to 3D colour conversion? Do you have a FAQ/
overview document somewhere that goes into slightly more detail on the
API?

################################################################################
On the general engineering front.

I've seen it mentioned in the posts Jon forwarded to me, but a C API
is more robust for code in a dynamic library. I generally prefer C++
myself, but you could have a core C API and wrap it with some very
very thing C++ classes as an option.

If you have
class ASCColorCorrection {...}
and a function
void ApplyASCColorCorrection(...., const ASCColorCorrection
&c, ...);
why not make that function with global scope a member function 'apply'
in the class?

If you start adding row and components strides as well as separate
buffer pointers the API, it will make for a complex set of calls to
apply a conversion/look to an image. I'd suggest a very lightweight
wrapper class for images that packages all that up, and you pass an
object of that class into the relevant functions. Given that you are
talking about supporting fixed component strides already, this
shouldn't make the implementation any more difficult and make the API
much tidier.

Something like...

class Image {
public :
/// single buffer RGB[A] ctor, packed rows, packed components
Image(float *data, int width, int height, int nComps);

/// single buffer RGB[A] ctor, unpacked rows, unpacked components
Image(float *data, int width, int height, int nComps, off_t
rowOffset, off_t componentOffset);

/// multi buffer RGB ctor, packed rows
Image(float *rdata, float *gdata, float *bdata, int width, int
height);

/// multi buffer RGB ctor, unpacked rows
Image(float *rdata, float *gdata, float *bdata, int width, int
height, off_t rowOffset);

/// a bunch of accessors
....

bool hasPackedComponents();
bool hasPackedRows();
};

void ConvertColorspaceToColorspace(const Image &img,

const std::string& inputColorspace,

const std::string& outputColorspace);

Then in Nuke I could go...

OpenColour::ConvertColorspaceToColorspace(OpenColour::Image(redBuf,
greenBuf, blueBuf, width, height), "BananaColourSpace",
"CheeseColourSpace");
And in a packed host you would go...

OpenColour::ConvertColorspaceToColorspace(OpenColour::Image(buffer,
width, height), "BananaColourSpace", "CheeseColourSpace");


-----------------------------


another half day of trawling the internet and reading through your
color.h header file and I have a better idea of what you are doing in
your library as is. I'm attaching my summary of the headers you sent
us which I have put up on our Wiki. I'm probably wrong on a few bits,
and would appreciate a quick glance over it to tell me where.

Two questions to start with,
- what is a Look?
- what is a Visualization as used by the Filmlook objects?


-----------------------------

Addl comments:
* implicit global state makes me nervous
* an arbitrary ApplyLut function that runs a lut, (refered to by file
name!) on an image.

The following need to be talked over/understood,

1. it needs a big tidy, some random functions should go, some
functions should be made members and so on, but they know that,
2. images are currently all packed RGBA/RGB, support for planar
images has been mentioned, no mention of support for separate RGB
buffers,
3. all that global state makes me nervous, I would prefer it
wrapped into some pimpl context object,
4. what some classes/entities are for is not clear, ie: Look and
Visualisation,
5. the relationship between a FilmLook, a named display and a
visualisation is unclear,
6. configuration is via external XML files, which we might need to
wrap.


--
Subscription settings: http://groups.google.com/group/ocs-dev/subscribe?hl=en


Jeremy Selan <jeremy...@...>
 

These are all excellent suggestions, thanks!

We hope to address all of these major issues in the next version of
the header (0.5.1).


--
Subscription settings: http://groups.google.com/group/ocs-dev/subscribe?hl=en