refactor: Barrage stream reading into Chunks

[niloc132]

Like #5552, applies some after-the-fact review of the design for reading Barrage/Flight stream, in anticipation of sharing this code with JavaScript clients.

• Removes unused BarrageChunkAppendingMarshaller
• Adds more error detail when stream contents don't match metadata
• Removes an unused parameter when parsing Flight messages
• Inlines width of various types into their readers
• Introduces an interface for reading data into chunks, and a factory interface to allow JS clients to supply their own implementations.

Partial #188

[niloc132]

We may want to replace these Function<BoxedPrimitive, T> interfaces with some replicated version to avoid unnecessary boxing of values that can never be null.

[niloc132]

In the interest of moving this patch along, I'm going to punt on this, the updated version is no more wrong than it previously was.

[nbauernfeind]

Although, we may want the transformers to "transform" null values to give better control to the future-feature of custom formatters. If this were the case then the boxing would be necessary.

[niloc132]

I believe that these already have the DH null values as primitives, so a null input is impossible at this time. T could certainly be null though for an output, depending on what kind of chunk is going to be written to (not controlled by this code).

[nbauernfeind]

I simply notice that we do not call the transformer if the value is the null value. As in, a custom transformer can't react to any null values. I see your point that we could use the deephaven null value and that using primitives will make it very clear that null is being represented in a non-boxed way. You're probably right that we should avoid boxing here.

[nbauernfeind]

Although, we may want the transformers to "transform" null values to give better control to the future-feature of custom formatters. If this were the case then the boxing would be necessary.

[nbauernfeind]

We need a detailed comment as to why we need to copy this. I suspect the reason is that the converted arrow schema references the new buffer? We may want to push the copying into BarrageUtil if that's the case because it's super common to assume that the byte buffer is temporarily immutable.

[niloc132]

It isn't quite about immutability, but the fact that the ByteBuffer is owned by python, and if py frees the underlying buffer we'll be reading garbage when trying to handle a later RecordBatch.

[nbauernfeind]

If possible I would like it more obvious why we need to copy here. (e.g. a comment related to what references get leaked)

[niloc132]

I'm not 100% that we need to copy in this case - technically it appears no since line 87 does a ByteBuffer.wrap(). Instead this is an attempt to be defensive in case a future impl is reading from a slice/etc of the ByteBuffer that came in over the wire.