oddity-ai / video-rs

Video readers, writers, muxers, encoders and decoders for Rust based on ffmpeg libraries.
Apache License 2.0
248 stars 31 forks source link

Add Support for Alpha Formats #49

Closed erlendp closed 2 months ago

gerwin3 commented 2 months ago

Unfortunately, I think this feature is a bit too complex for video-rs. I requires a lot of custom handling throughout to make it work. It would be great if there were some ergonomic way of supporting more pixel formats both for decoding and encoding, and if there's a design that works, then implementing this would be easier.

erlendp commented 1 month ago

Unfortunately, I think this feature is a bit too complex for video-rs. I requires a lot of custom handling throughout to make it work. It would be great if there were some ergonomic way of supporting more pixel formats both for decoding and encoding, and if there's a design that works, then implementing this would be easier.

So I agree that it'd be nicer to have a robust method for handling encode / decode formats, but I disagree that this PR is adding much more complexity to the existing approach. Conceptually, it's just extending the existing hard-coded format so it doesn't throw away a channel of useful data where applicable, and would be no more difficult to replace with a well thought out pixel format selection design at a later stage.

That said, I can see where you're coming from with wanting to hold off - there's always overhead with reviewing, testing, releasing, etc. I'll probably end up switching directly to the ffmpeg bindings, as I'll likely need more flexibility than this library is likely to be able to provide in the future.

gerwin3 commented 1 month ago

So I agree that it'd be nicer to have a robust method for handling encode / decode formats, but I disagree that this PR is adding much more complexity to the existing approach. Conceptually, it's just extending the existing hard-coded format so it doesn't throw away a channel of useful data where applicable, and would be no more difficult to replace with a well thought out pixel format selection design at a later stage.

I think most of the complexity is in the unexpected changes to pixel format in many places without the user knowing. I agree the PR itself is not problematic, it's just there's no way to do this correctly due to the hardcoded pixel format currently.

That said, I can see where you're coming from with wanting to hold off - there's always overhead with reviewing, testing, releasing, etc. I'll probably end up switching directly to the ffmpeg bindings, as I'll likely need more flexibility than this library is likely to be able to provide in the future.

Yeah that makes sense. The aim of this library is mostly for simple use cases and there's a lot abstraction