Re: [boost] [gil::io] Feedback for scanline_read_iterator

Subject: Re: [boost] [gil::io] Feedback for scanline_read_iterator
From: Christian Henning (chhenning_at_[hidden])
Date: 2013-02-24 16:07:51

Hi Michael,

>> I moved the buffer creation internally.
>>
>
> I think this might be even worse unfortunately, if the iterator is copied, such
> as with (*i++) it's going to make a full copy of the buffer. Now that readers
> have a begin and end maybe the reader could own the buffer?

I see two ways, apart from moving the buffer into the reader. I could
use a shared ptr or the user has to pass the buffer into the
constructor. I lean towards the shared pointer. Do you have an
opinion?

>
>>
>> Skipping can be different for different formats. jpeg for instance
>> still reads the scanline when skipping. For now, I think, we are good.
>>
>
> Sure that makes sense, but the interface lies IMO. If I have jpeg and I call
> skip( buffer, 0 ) it's not going to be skipping over scanline 0 (unless it
> happens to be the first call to read/skip), it's just skipping forward one
> scanline, the second parameter is unused.
>
> Similarly the TIFF reader seems to read skipped scanlines unnecessarily as skip
> forwards to read but the TIFF interface supports reading any scanline you want
> directly with TIFFReadScanline.

As far as I can tell tiff needs the row number to be read and also
tiff rows cannot be read in random order. Jpeg also needs to be read
in order which means there is no skipping.

Now, because of that and to ensure a common interface among all
scanline_readers I need to pass the pos into the skip function.

> iterator_facade is super duper simple. You just implement a handful of private
> member functions, specify the appropriate iterator tag and it generates the rest
> of the iterator for you with no hassles. I highly recommend trying it.

Ok, I agree it's foolish of me not to make use of interator_facade. I
have committed the new version.

Thanks again for your insight.

Christian