Added support for OpenColorIO displays with the new --ociodisplay flag.

[mjmvisser]

The new --ociodisplay flag applies color transforms from an OCIO display/view combination. Optional keywords are "from" (source colorspace), "looks" (optional looks overrides, supports empty string to disable looks), and the same "key" and "value" keywords as --ociolook.

I think it would be useful to be able to request the default display and view, but I'm not sure what the syntax should be. The best I could come up with was using empty strings to indicate defaults:

--ociodisplay "" ""              # default display and view
--ociodisplay "sRGB" ""          # default view for the display named "sRGB"
--ociodisplay "sRGB" "FilmLook"  # explicitly specified display and view

I'm not sure that's good syntax, so it's commented out at the moment. Feedback is appreciated!

[lgritz]

I'm hoping Jeremy will jump in and comment.

Do you think that this merits a new ImageBufAlgo::ociodisplay() function, which then oiiotool will more plainly wrap rather than having so much logic of its own? (As we've done with ociolook and colorconvert lately?)

[mjmvisser]

I think it's useful to be able to specify a default display and/or view, though.

[mjmvisser]

Re: new ImageBufAlgo::ociodisplay() function

I'll do it in another branch this week when I have a few hours free, then we can see which is cleaner.

[mjmvisser]

It seems that oiiotool doesn't actually use ImageBufAlgo::ociolook at the moment. Only py_imagebufalgo.cpp references it. oiiotool.cpp:action_ociolook creates a ColorProcessor then applies it with ImageBufAlgo::colorconvert.

I'll switch oiiotool over to using ImageBufAlgo::ociolook and submit a new pull request.

[lgritz]

I think instead, we can just make ociodisplay take the from and looks parameters directly as std::string, and it all works out, no need to extract it from the object.
I'll make that change on my end before merging.

Also, I'm tempted to reverse the relative order of 'from' and 'looks', because in the existing ociolook, 'looks' comes before 'from'. Do you have any thoughts on that?

[lgritz]

I'm also going to call them 'key' and 'value' to match the existing ociolook call.

[mjmvisser]

I made looks an object instead of std::string because there is a semantic difference between None and an empty string. None means "do not override looks", while an empty string means "override looks with empty string", i.e. remove all looks. And std::string cannot encode a null, so hence the object and extract.

Line 377 is comparing from to None (object() is None in boost::python, not at all obvious and I should have commented that).

ImageBufAlgo::ociodisplay has looks defined as a const char * instead of a std::string for the same reason.

It seemed simpler than adding a boolean parameter called "override_looks", and a default value of None is pythonic.

edit: Agreed on relative order, consistency is good. I can't remember exactly how I arrived at the current ordering.

[lgritz]

OH, I get it now. I didn't quite grok that distinction in the C++ API, so didn't understand what the Python was up to. I understand and will put it back to your way before merging.

[lgritz]

All LGTM. I'm going to commit this to master as soon as I get the ok from Jeremy's quick look.

[lgritz]

Squashed, documented, and merged. If there's a problem, we can come back to it.