search

ralfbiedert / openh264-rs

Idiomatic Rust wrappers around OpenH264.
69 stars 35 forks source link

Added trait for RGB data input #48

Closed joe-saronic closed 6 months ago

joe-saronic commented 6 months ago

I recently came across a situation in which I had RGB data encoded in a slightly different format from the one YuvBuffer currently expects. The processing already uses a callback internally, so I factored it out into a public trait, with a backwards compatible implementation for [u8; N].

I also added some implementations for the cases I care about with u32 data (coming in from a cairo Surface).

Two other tweaks:

  1. YuvBuffer now checks for evenness of dimensions during construction, not every time it is populated
  2. User is now responsible for making sure that the source data has the right format, instead of checking in YuvBuffer
ralfbiedert commented 6 months ago

I like the direction, I'd just have some minor nits about the trait methods (e.g., pixel -> pixels_f32 and maybe adding pixel_u8), and the trait implementations (e.g., not sure if implementing for [u8;N] is good idea (e.g., maybe a thin newtype pattern would be better to avoid accidentally passing the wrong type).

That said, I'll merge this PR, the cleanup can be a separate PR.

Thanks!

joe-saronic commented 6 months ago

Thanks! I would be happy to do the fixes, but it's probably faster if you do. When can I expect the new version to be released?

joe-saronic commented 6 months ago

Also, if you add pixel_u8, I would vote for it to be optional. The nice thing about only supporting f32 is that I can have random input like RGB with 16 bits for G or something. If I have to convert to u8 first, a lot more information is lost.

ralfbiedert commented 6 months ago

I would be happy to do the fixes, but it's probably faster if you do. When can I expect the new version to be released?

I'm having a look right now, if everything goes well soon™️ (~few days).

joe-saronic commented 6 months ago

Thank you!

ralfbiedert commented 6 months ago

I just pushed some changes regarding the YUV / RGB buffer handling to master, feel free to have a look and comment.

joe-saronic commented 6 months ago

I added some comments to the last commit. I like the approach that you took with this. It's got all the functionality I want, but much cleaner.

ralfbiedert commented 6 months ago

Version 0.6 has been released. It contains this PR, and some moderate API changes. It should not take more than a few minutes to upgrade to the new API surface. Feel free to file a follow-up PR / bug report if there are issues.

joe-saronic commented 6 months ago

Thanks! I'm already purging much of my code to use the new API.

ralfbiedert commented 6 months ago

Ah, the Panics line was probably an old copy-paste error. Thanks!

joe-saronic commented 6 months ago

You're very welcome.