Let PacketWriter::write_packet use buffers convertible to Cow

[AlexTMjugador]

As pointed out in issue #19, in some cases it may not be desirable to heap allocate Ogg packet contents, but the current parameters of the write_packet method impose a heap allocation no matter what.

However, the PacketWriter struct doesn't actually need to own the packet data, as long as it stays in memory unmodified during its lifetime. The Cow smart pointer that the standard library provides captures this requirement more precisely, and provides for more elegant, flexible and potentially efficient client code.

Therefore, substitute Box for Cow in the aforementioned code. Boxed slices can still be used with this new design, although with a different syntax, which breaks backwards compatibility.

[est31]

Thanks for the PR! I wonder if it's possible to have something more general than Cow. Maybe AsRef<[u8]> or something?

[AlexTMjugador]

I wonder if it's possible to have something more general than Cow. Maybe AsRef<[u8]> or something?

This sounds like a good idea, thanks! However, after swapping the generic Into<Cow<[u8]>> parameter for AsRef<[u8]>, I found the result less ergonomic, because the write_packet method would need to always take a reference to a byte buffer, and not be able to accept an owned buffer like Vec<u8> (otherwise Rust would drop the maybe owned AsRef<[u8]> parameter at the end of the method body, which does not compile, because it needs to be converted to a byte buffer by calling as_ref, which borrows it. It may be possible to workaround this by adding additional type parameters to the struct or something like that, but that sounds like a fair amount of redesign work for not so much gain, and caller code would probably need to always provide buffers of the same type.)

On the other hand, with the current approach, the caller can just convert whatever type that implements AsRef<[u8]> beforehand to a byte slice by calling as_ref on that type, and things would still work fine because a Cow can then be constructed from that borrowed byte slice. So I'd argue that it's still flexible enough to accept a wide variety of byte buffers, although as I explained before the lifetime requirements are a bit longer than a user might expect, as it is necessary to call as_ref relatively "early", so the borrow lasts for long enough.

Another advantage I can see from using a Cow is that, in the (very?) unlikely case that this library is changed so there is a need to modify packet data before writing it, a Cow can also support that as efficiently as possible, because if the user already provided a Vec<u8> to the method, its memory buffer will be reused without any additional cost.

[est31]

Good point about the need for a generic param. The PR should be good now but I won't merge it for now because it is a breaking change of the API. I want to make a new release of the ogg crate once the 2021 edition lands though.

[AlexTMjugador]

That's understandable, thanks. However, right now it's bugging me a bit that the CI build for Rust 1.27.2 does not pass, as I've developed this PR using the latest Rust version and used some syntax that simply didn't compile back then. Do you plan on bumping MSRV on the next release? Should I continue to fix compilation with this older Rust version?

[est31]

@AlexTMjugador no, the MSRV will be increased to 1.56.0.