I'm going to have to (I hope you don't mind!) disagree with you on what is considered the "image" vs what is extra information.
I would say that full_width and full_height are the resolution of the image. They are the area that you care about. The data window (width/height/x/y) is just a way for the image to say "I know you care about stuff outside this area, but trust me - there isn't anything here, so I'm going to be nice to your storage/io needs by not actually storing the pixel data for that area". It's also saying "I know you don't really care about these pixels outside your image here, but I'm going to hold onto them for you just in case". You mentioned an image with a pixel domain of 0,0 -- 15,15, embedded in a virtual canvas with the domain -10,-10 -- 25,25. Generally, in my experience, the display window has the origin at (0,0). For example, an EXR that I've been using to test this has the following: dataWindow : (511,-468) -> (1536,2023) displayWindow : (0,0) -> (2047,1555) The "image" here, as I think of it, is the 2048x1556 display window, with the origin at (0,0) Another example might be a CG render that has had an AutoDOD applied. Each frame has the same sized displayWindow, but differing dataWindows. The data window will also move around the display window, based on where the CG happens to be. If I were to take this image sequence and pass it through oiiotool to convert it to, say, jpegs, we'd end up with an image sequence with every frame being a different size. If I were doing this from scratch, I would probably have named width/height/full_width/full_height slightly differently. I would have gone with: full_width -> width full_height -> height width -> data_width height -> data_height etc I think that the default behaviour should be to generate images that all have the same total image size (ie full_width/full_height). There should then be a function (and flag in oiiotool) to say "I actually want to make the image just the size of the data window", which will go back to how it is currently working. As for the stash_spec() method, I called it that because the comment on pretty much every "m_spec = userspec;" line was "stash the spec". Maybe "set_spec" would be a better name. It was intended as a helper function to stop the same code needing to be put in evert image writer that didn't support data windows. Hugh Macdonald nvizible – VISUAL EFFECTS [email protected] +44(0) 20 3167 3860 +44(0) 7773 764 708 www.nvizible.com On 5 Oct 2011, at 08:10, Larry Gritz wrote: > I replied already on the ticket, but now I've thought more about the issues > and so I'll follow up here. > > What should we do for formats that do not support differing 'data' > (width,height) and 'display' windows (full_width, full_height)? And > similarly, for formats that don't support nonzero origins > (x,y,full_x,full_y)? There are two possible approaches: > > 1. Write the original width x height pixels, but just omit the > full_width,full_height metadata. > > 2. Write an image of full_width x full_height, padding with black pixels. > > I suppose it depends on philosophy of what the windows "mean". I think of > width,height (aka data window) as the *actual* image resolution, and > full_width,full_height (and x,y,full_x,full_y) as additional, optional > information that says "here's where I think those pixels should be placed, > possibly in a differently-shaped virtual canvas or displayable window". The > alternate approach, I suppose, is to consider the display window to be the > "real" image, and the x,y,width,height to be extra information saying "as a > shortcut, I'm only going to give you a subset of those pixels, or > alternately, extra pixels extending beyond the border." Both are equally > valid and true, but we may differ in which we think gets at the core > relationships. > > I'm inclined to vote for #1, both because of my philosophical leaning above, > and on the grounds that the best policy is "write the pixels the app hands > you, and change as little else as possible." Changing outside-display-window > pixels from "undefined" to "black" is a big leap in my book, as would be > dropping overscan pixels, which I think are the things that happen in Hugh's > patch. I don't think that's something an ImageOutput ought to decide to do > on its own. > > In fact, I claim that all the ImageOutput implementations already have the > best low-level behavior -- they will write the actual pixels passed from the > app, and save the metadata about the origin and "full" (display) window if > the output format supports it (otherwise silently dropping it). So I'm > leaning toward believing that none of the output plugins need to change at > all (except for possibly adding new supports() tags so they can communicate > their support for origins and/or full windows, that's definitely a handy > idea). > > I'm willing to believe that a behavior change at the ImageBuf or oiiotool > level is appropriate. Is ImageBuf really causes a problem in this case? I'm > not sure I'm convinced yet. > > Really, I think that what Hugh wants is to beef up the behavior of oiiotool > to more gracefully handle operations on multiple images that do not all share > a common resolution and offset. In fact, it may also be awkward for non-zero > offsets and mismatched display and data windows, even if all images match > each other. Let's fix that, I think it more directly gets us where we want > to go. As part of this, I think it's perfectly reasonable to have oiiotool > options that can choose among different behaviors when there are mismatches, > or when outputs are directed toward files with formats that don't support > virtual canvases ("pad black pixels" may be a perfectly reasonable choice > among those options). > > Sorry, Hugh, believe it or not, I did not wake up this morning with the goal > of making all of your pull requests moot! In fact, I really liked this patch > when I first saw it, was ready to give it a "LGTM", but the more I think > about it, the more I believe it's attacking the problem at the wrong level of > abstraction. > > -- lg > > > On Oct 3, 2011, at 6:34 AM, Hugh Macdonald wrote: > >> We discovered last week that, if we took an EXR with a data window that was >> not it's display window (let's say, for example, that the display window is >> 2048x1556, but the data window has the soundtrack area cropped off, and is >> 1828x1556, with an xoffset of 220), if you used oiiotool to convert this EXR >> to a DPX, the DPX would end up the size of the data window and not the size >> of the display window. >> >> I've submitted a pull request (Issue #176) for a fix for this. It works as >> follows: >> >> ImageOutput::supports() now has an extra feature that it may expect - >> 'datawindow'. Currently, only EXR, RLA and TIF return true for this. >> >> In the open() function for each of ImageOutput's subclasses, instead of >> setting 'm_spec = userspec;', they now call 'stash_spec(userspec);'. This >> sets m_spec, but also, if supports('datawindow') returns false, sets >> m_spec.width = m_spec.full_width, m_spec.height = m_spec.full_height, etc. >> >> Lastly, in ImageBuf::write(), if the ImageOutput that it's going to write to >> does not support('datawindow'), and the ImageBuf's width/height/etc don't >> match full_width/full_height/etc, then it calls ImageBufAlgo::crop() to >> create a new ImageBuf object with the right amount of data in, and then >> calls write() on the new ImageBuf object. >> >> >> Any thoughts? I'm thinking that the method for padding/cropping could be >> more efficient, but I didn't want to be changing the ImageBuf in place, just >> in case it had wider-ranging effects. >> >> Hugh Macdonald >> nvizible – VISUAL EFFECTS >> >> [email protected] >> +44(0) 20 3167 3860 >> +44(0) 7773 764 708 >> >> www.nvizible.com >> >> _______________________________________________ >> 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
