Is it possible to replace MediaSourceStream with generic M: MediaSource?

[compilerintrinsics]

Instead of taking in MediaSourceStream, is it possible to have decoders generically take in a M: MediaSource trait instead? (Or possibly Box<dyn MediaSource>, although I prefer the generic version because of fewer pointer indirections). The reason being, the underlying MediaSource implementation may already be buffered or is entirely in-memory, causing unnecessary copy into the buffer within MediaSourceStream.

The existing MediaSourceStream can be repurposed as MediaSourceStream<M>, a buffered wrapper type over MediaSource types, so that the user can decide for themselves whether they require buffering (pay only when needed).

[pdeljanov]

I think this is a topic that could go very deep, but...

The short answer is that from my experience, I don't think there will be much of a gain in performance. Nothing measurable at least. Memory copies are extremely fast and optimized, and we are only copying tens of MB of memory. Removing a single branch from a Decoder hot-path would yield more benefits.

---

The long answer is that it's a complicated problem...

MediaSourceStream is basically a BufReader with extended capabilities.

The motivation behind making MediaSourceStream take ownership of a Box<dyn MediaSource> is so the overhead of an indirect call is only paid when buffering new data. In this way, the cost of that indirect call is amortized over all the read calls on MediaSourceStream it takes to consume the newly buffered data.

If we made everything that used MediaSourceStream generic over MediaSource instead, that would solve the indirect call issue, but it would also just increase the complexity of everything else.

MediaSourceStream also provides the following invaluable capabilities:

1. The ability to grow the underlying buffer as needed
2. The ability to seek within the buffered data without seeking the underlying source
3. Optimized single, dual, triple, quad, and octo-byte read functions
4. Exponentially growing read-ahead chunk size to reduce amount of buffered data when performing many seeks

There are basically no cases where none of these features are used, so even if we went with the buffered wrapper approach, the benefits would be lost since everything would use the buffered wrapper.

Therefore, to see any advantages, MediaSource would need to be extended to support these capabilities. I feel this would be rather complicated for any media source beyond just a big buffer in memory containing the file. So the trade off of using MediaSourceStream with an extra copy, I feel, is worth it.

[compilerintrinsics]

To my understanding, adding generics shouldn't bring much more complication except for propagating the type parameters since the same methods are gonna be called; there should be almost no change to the actual logic (but I may be wrong).

In my opinion, the principle of pay-for-only-what-you-use is just too attractive and aligned-to-Rust to pass up. In a sense, this is "free" performance that users at a higher level have to leave on the table because the choice had been made for them, even though they might have been in a better position to make the call themselves.

[pdeljanov]

To my understanding, adding generics shouldn't bring much more complication except for propagating the type parameters since the same methods are gonna be called; there should be almost no change to the actual logic (but I may be wrong).

For the most part, yeah.

Thinking about this further, the probable way forward would be to make MediaSource a trait that covers the public interface of MediaSourceStream. All references to MediaSourceStream across all crates would then be changed to a generic with the MediaSource trait bound.

The existing MediaSourceStream then just becomes the batteries included default implementation of a MediaSource that would cover most use-cases.

In my opinion, the principle of pay-for-only-what-you-use is just too attractive and aligned-to-Rust to pass up. In a sense, this is "free" performance that users at a higher level have to leave on the table because the choice had been made for them, even though they might have been in a better position to make the call themselves.

The drawback of course would be the complexity of new MediaSource implementations and whether it's worthwhile and practical to bother.

I think one application where this could yield a theoretical benefit is network streaming. Presumably instead of copying newly received data into a buffer the preexisting buffers could be used, but I'm not entirely sure if that's reasonable.

Would love to hear others' opinions on this.

[hasezoey]

i think relevant to this issue would be #260, i ended up doing a buffer before MediaSourceStream to reduce sys calls, which ended up with more buffers than would likely be necessary. though at the current i dont see much performance problems with the extra buffer and copy from / into buffer