Why is `ByteVector.toByteArray()` deprecated/internal?

[smoores-dev]

Howdy! This library is fantastic, I can't thank you enough for maintaining it.

Let's say I'm extracting the cover image from an audio file to return in a node.js Response. I'd like to be able to pass the bytes from that image directly to the Response constructor.

It seems that the only way to do this is via picture.data.toByteArray(), which is totally fine. But... why is this marked as internal/deprecated? Is there some other, more idiomatic/safe way to approach this that I'm missing?

[benrr101]

Hey @smoores-dev - I'm sorry I didn't look at this sooner. I'm going to try to be more proactive in keeping up with this repo going forward. It gives me joy to work on it, so I just need to make sure I get time to do it.

For ByteVector.toByteArray() ... The original implementation of this port is based on TagLib# which makes heavy usage of the ByteVector type, which I think is a holdover from it being a port of TagLib which was written in C++ which didn't have a built-in dynamic array type. The original implementation of ByteVector copied the array a lot. When I rewrote the class a couple years ago, I made it so that ByteVector is copy-on-write, and operations like subarray operate on a view of the backing Uint8Array. This was a huge improvement to performance for stuff like reading portions of files, etc. But if someone were to directly access the backing Uint8Array, they could write to it and totally break the copy-on-write functionality. The toByteArray is such that it just returns the backing array with no modification. If that backing array is a view of another array, something as benign as:

myByteVector.subarray(1,5).toByteArray()[0] = 1

would cause the value of myByteArray to change possibly unintentionally. Ie, breaking copy-on-write behavior.

As you rightly noticed - whenever you're dealing with external APIs ... they don't know what a ByteVector is, they only work with Uint8Array. So, how do you get a Uint8Array from a ByteVector? toByteArray of course! Even I ran into this - there's one instance of using toByteArray when actually writing changes to a file. But I foolishly hadn't considered someone else would need to do the same thing as me 🤦‍♂️

So I think what I should do for this is introduce an toByteArrayInternal that just directly returns the backing array. But change toByteArray to do a copy and then return the copy of the array. This would be kinda expensive, but I think I still need to maintain the boundary between node-taglib-sharp and whatever is using it to ensure internal states don't get corrupted. But I'm open to any suggestions - I'm primarily a C# developer, so maybe there's a more nodejs-friendly way to do what I'm trying to do.

[smoores-dev]

Hmmm yeah this is interesting. Maybe the best way forward, actually, would be to have ByteVector implement ReadableStream, backed by the ReadableStream from its TypedArray. That gives you the ability to expose read-only stream access for, e.g., streaming to an HTTP Response, without exposing write access to a shared data array and risking corruption.