tuffy / bitstream-io

Library for reading/writing un-aligned values from/to streams in big-endian and little-endian formats
Apache License 2.0
52 stars 20 forks source link

FromByteStream quite limited because only ByteRead functions can be used #14

Open tp-m opened 11 months ago

tp-m commented 11 months ago

Apologies if this is a bit all over the place, but just wanted to quickly drop off a note on some issues I ran into whilst trying to use FromByteStream.

The following things I sometimes need when parsing certain types of data (from a slice of bytes):

When using ByteReader directly that's no problem, since we can get access to underlying reader.

However, inside a FromByteStream implementation we only have the ByteRead trait to play with, which is very limited.

I wonder whether it would make sense to define ByteRead as pub trait ByteRead: Read?

In addition it would also be nice to be able to peek without consuming the data, but I guess that's independent.

cc @sdroege

tuffy commented 10 months ago

The trouble with ByteRead being a subtrait of Read is the name collision with the read() method, so that doesn't quite work.

But I've pushed out a new 1.9 version with reader_ref() and writer_ref() methods in the ByteRead and ByteWrite traits to get boxed references to the underlying reader/writer which you can downcast to concrete types, if necessary. Feel free to give it a try and let me know if you find them helpful.

Having a peekable reader that also implements reader methods would be a good idea, but I'll have to think of the best way to implement it.

sdroege commented 10 months ago

Those don't have to be boxed. You can just return a &mut dyn Read etc there and not have one heap allocation per call :)

sdroege commented 10 months ago

I'd suggest yanking 1.9.0 fast and making this change (without Box) as 1.9.1. Technically it would be a breaking change but nobody should've depended on the Box version of the API yet.

sdroege commented 10 months ago

See https://github.com/tuffy/bitstream-io/pull/15

tp-m commented 10 months ago

Many thanks @tuffy !