It's not just "slightly related" -- yesterday I implemented exactly what you
requested, and in so doing (needing to specify channels) realized the
discrepancy between the way ImageInput specifies channel ranges and how ROI and
ImageBuf do it, leading to my email.
At present, the ImageCache always reads and caches all channels in a file, on
the theory that you'll generally need all at once, and even if you ask for a
subset now, chances are high that you'll want other channels in the same tile
fairly soon (while it's still in cache). So while we could imagine modifying
the IC to store fewer channels in each tile (1? 4?), it's not immediately
obvious that the speedup in the "ask for a subset of channels" case would not
come at a big cost to the more common "all channels" case. If it becomes
common to have files with huge number of channels, and apps that only care
about a very few of them at a time, we'll certainly want to re-evaluate our
strategy.
OK, if there are two common use cases (all channels, and one channel), then
what would you think of my specifying that the function retrieves a minimum of
one channel? That way, retrieving channel 2 only would have previously
specified firstchan=2, nchans=1, and the new interpretation chbegin=2, chend=1
falls into the "1 channel minimum" case and also works the same as before.
-- lg
On Nov 14, 2012, at 6:30 AM, John Haddon wrote:
> Hi,
>
> On a slightly related note, is there any chance of an
> ImageCache::get_pixels() variant which specifies a channel range, in addition
> to the ImageInput methods under discussion? I'm using OIIO for a simple
> compositing tool which prefers to operate channel-by-channel - the ImageCache
> is great for speeding up repeated access to the same images but currently
> forces the loading of all the channels at once which isn't ideal for my
> scenario. If such a variant was supported, is the implementation of the
> ImageCache such that all channels would be loaded anyway, and get_pixels()
> would just be skipping data, or would channels only be loaded as and when
> requested?
>
> As to the original question, I personally would be more than happy to deal
> with a change to specifying channels as a begin/end range in return for API
> uniformity, but I don't think the argument that it's unlikely to break code
> is a particularly strong one. It seems to me that one of the most natural
> uses for those calls it to request one channel at a time for a process that
> prefers to operate that way, in which case firstchan!=0 and nchans==1 would
> be rather likely.
>
> Cheers...
> John
>
> ________________________________________
> From: [email protected]
> [[email protected]] on behalf of Michel Lerenard
> [[email protected]]
> Sent: Wednesday, November 14, 2012 1:04 AM
> To: [email protected]
> Subject: Re: [Oiio-dev] API question with channels
>
> I did noticed it, but it doesn't bother me that much (i am using these
> functions). I thought there was a good reason behind it.
>
> I think it would be better to harmonize the parameters, and drop the
> range version. Having both will complicated things, the signatures being
> the same we'd need to have different function names.
> I'm not sure people using OIIO have all switched to 1.1.0 (or yet know
> the version is out), so I vote for the solution you're proposing: change
> things right now. Hopefully this API modification will be seen as a
> feature of the new version.
>
> As for breaking this code, I hope people using the library have
> subscribed to the ML and will notice this thread.
>
> On 11/14/2012 07:13 AM, Larry Gritz wrote:
>> Does it bother anyone that the ImageInput::read_* functions, the varieties
>> that let you specify a channel subset, look like this:
>>
>> virtual bool read_tiles (int xbegin, int xend, int ybegin, int yend,
>> int zbegin, int zend,
>> int firstchan, int nchans, TypeDesc format,
>> void *data, stride_t xstride=AutoStride,
>> stride_t ystride=AutoStride,
>> stride_t zstride=AutoStride);
>>
>> The x, y, and z extents are given as [begin,end) ranges, but for some reason
>> that escapes me now, the channel subset is given as a first channel and
>> number of channels.
>>
>> ROI and ImageBuf::get_pixel_channels both specify channel subsets as
>> [chbegin,chend), in keeping with how we specify spatial ranges everywhere
>> else.
>>
>> I hadn't fully noticed this discrepancy until today, and now that I have, I
>> can't unsee it or stop thinking about it. It's bugging the crap out of me.
>>
>> My inclination is to apologize profusely for tagging 1.1.0 immediately
>> before contemplating an API change, ask for a do-over, quickly change it to
>> chbegin/chend, and vow not to mix metaphors for in any future API calls
>> (always begin/end ranges).
>>
>> I'm not sure how often anybody is actually using the channel subset versions
>> of these calls. But note that specifying firstchan=0,nchans=nchannels gives
>> the same behavior as specifying chbegin=0,chend=nchannels. So it's very
>> likely that this wouldn't break anyone's code.
>>
>> Which is the lesser evil, changing the API but having a uniform way to
>> specify ranges (spatial or channel), or allowing it to continue with
>> different ways in different interfaces and different methods for spatial
>> versus channel ranges?
>>
>> Arguments pro or con?
>>
>> --
>> 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
>
>
> _______________________________________________
> 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
