CCing oiio-dev.
I'm willing to make valid_file required. Like you said, it is possible to look
like a valid file but still fail open, just like it's possible to open but fail
a subsequent read_scanline (if the file is corrupted or truncated).
As for create/open...
The status quo was that create() makes an ImageInput that *can* open the file,
then open() tries to open the file. My proposal was to add a variety of
create() that will optionally return an already-opened file, since a common
idiom is to create and immediately open.
Chris Foster proposes that create *always* open the file if it can, more or
less deprecating the use of open for client software (though it would still
exist in the plugins, so it can be explicitly called by create itself). In the
rare case that they don't want the open file, they can always close it
explicitly.
I can think of two reasons why you'd still want to separate them: (1) so you
can ask for supports("blah") on the II that hasn't yet been opened, without the
expense of the open, or on a generic II of the right type (think:
create("tiff")); and (2) so you can call the open-with-config variety:
open(name,newspec,config). The latter could be accomplished by having yet
another call signature of create() that takes the config, but that would make
it harder to have client software with special cases for individual formats
(i.e., special config for just one type).
I'm also hesitant to break back compatibility, but I don't know if I should be.
Does everybody prefer the "just call create" idiom and is happy to break
compatibility?
Opinions?
-- lg
On Apr 4, 2012, at 5:33 AM, Chris Foster wrote:
> Hi Larry,
>
> On Tue, Mar 27, 2012 at 6:05 PM, Larry Gritz
> <[email protected]>
> wrote:
>> In the continuing quest to make create/open more efficient, here are
>> two minor API changes:
>>
>> 1. In almost all uses, an app calls ImageInput::create() to get an
>> II*, then immediately calls in->open() to open the file, which, if you
>> think carefully about the fact that create() does an open to verify
>> that the II is capable of opening the file, means that open() ends up
>> getting called twice. Very wasteful. So the first change is that we
>> change the ImageInput::create() function prototype to include a
>> do_open flag -- if true, it returns the file already opened, not
>> needing a separate call to open(). If false (and this is sometimes
>> useful), it behaves as before, returning an ImageInput* that can be
>> opened, but is not presently.
>
> I like this. However, can we simplify the API by always returning an
> open ImageInput and require that apps simply call close() if they want
> it closed? For some reason I prefer that to the do_open flag, I think
> it's a bit simpler (and perhaps clearer to read in user code).
>
> Oh, but the above suggestion isn't source compatible which is a rather
> big downside... unfortunately I haven't thought of a good compromise.
>
>> 2. To further streamline, we add a new optional method,
>> ImageInput::valid_file(filename), which returns true if the named file
>> appears to be a valid file of that format (in a way that's less
>> expensive than doing a full open and header read -- for example, just
>> checking the first few bytes that contain a "magic number" or
>> whatever). Not all plugins supply this function, and it's not
>> required; the ones that do must return true for
>> supports("valid_file"). I've included implementations for OpenEXR and
>> TIFF that are considerably less expensive than a full header read.
>
> Yes, again I think this is a good idea in general but I second Colin's
> view that it should be provided uniformly across for all ImageInputs
> rather than be optional.
>
> With that in mind, how about these semantics:
>
> * If valid_file() returns true, the file *may* be a valid file (after all,
> even after reading a valid magic number the full header read may still
> fail).
> * If valid_file() returns false, the file cannot possibly be valid, and
> open() is guaranteed to fail.
>
> With these semantics valid_file() can have a sensible default
> implementation of simply returning true. The name might need to be
> changed - some alternative ideas: maybe_valid_file(), guess_valid_file()
> or invert the test and call it invalid_file()?
>
> ~Chris
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/OpenImageIO/oiio/pull/265#issuecomment-4952378
--
Larry Gritz
[email protected]
_______________________________________________
Oiio-dev mailing list
[email protected]
http://lists.openimageio.org/listinfo.cgi/oiio-dev-openimageio.org
