[pdal] questions about iterator and buffer semantics

[Chris Foster]

Hi all,

I've been digging through the code to solve a segfault when the las
reader is connected to a mosaic filter with a small chunk size.
Here's a pipeline which will crash pcpipeline when executed with
--chunk-size 100 (anything smaller than the number of points in the
file should lead to the crash).

<?xml version="1.0" encoding="utf-8"?>
<Pipeline version="1.0">
    <Writer type="drivers.faux.writer">
        <MultiFilter type="filters.mosaic">
            <Reader type="drivers.las.reader">
                <Option name="filename">
                    autzen-thin.las
                </Option>
            </Reader>
        </MultiFilter>
    </Writer>
</Pipeline>

I've submitted a pull request which fixes the problem
(https://github.com/PDAL/PDAL/pull/146), but without knowing the
intended iterator semantics it's hard to know whether my fix is in
keeping with the overall design.  So, here's some questions:

Is StageIterator::read() allowed to be supplied with multiple buffers?
 That is, I'm imagining something like

PointBuffer buf1(...);
StageSequentialIterator* iter = someStage->createSequentialIterator(buf1);
PointBuffer buf2(...), buf3(...);
iter.read(buf2);
iter.read(buf3);

If no, then this bug is really a bug in Mosaic, which creates
temporary buffers inside a loop and copies results from those into the
output buffer.  In this case, it's also arguably a deficiency in the
API, which allows various buffer arguments to read().

If the user *is* allowed to pass multiple buffers to iter.read(), then
why do the iterator constructors require a buffer?  As far as I can
tell, the only place that StageIterator::getBuffer() is called is by
the las reader random iterator, and I just removed that usage in the
above pull request!


On a slightly different topic, I'm curious about the reason that
readBufferBegin() and readBufferEnd() are required, since these always
seem to simply bracket readBuffer().  If this is true, it seems like
these just lead to a more stateful and confusing API.


I'm still very much liking the library, despite the above nitpicking :)
~Chris