-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
encoder: Add V4L2 Stateful encoder. #75
Conversation
962bc40
to
7cafa42
Compare
26fea55
to
bb95bd1
Compare
Hey, I'd like to start reviewing this, do you think you could rebase on the latest ToT? |
Hey, sure, I'll do this today 😄 |
The PR is now rebased 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still reviewing the bulk of the CLs, but overall looks great. A few inline comments, basically would love if you could elaborate a bit more on why some of the CLs are necessary.
@@ -424,12 +424,18 @@ where | |||
let picture = picture.render().context("picture render")?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which parser fails without this? Please add this to the description so we have a way to test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brought up offline.
src/backend/v4l2/encoder.rs
Outdated
Unsupported(UnsupportedError), | ||
|
||
#[error(transparent)] | ||
InitializationError(#[from] InitializationError), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want to split this variant into its own error type - since create
only returns InitializationError
(with one exception that we can add to the new type), it removes to need for the double map_err
while also making our errors more local.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be better now. PTAL 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huge thanks for the review!
@@ -424,12 +424,18 @@ where | |||
let picture = picture.render().context("picture render")?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brought up offline.
src/backend/v4l2/encoder.rs
Outdated
Unsupported(UnsupportedError), | ||
|
||
#[error(transparent)] | ||
InitializationError(#[from] InitializationError), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be better now. PTAL 😄
709c5d5
to
12a11e9
Compare
This change moves the code responsible for generation of test frames to encoder common module and modifies the logic to use lambda to deduplicate the generation code for both NV12 and P010. Futhermore the fill_test_frame was renamed to fill_test_frame_nv12 to better reflect its function.
This commit exports the code that generates t value using timestamp and max number of frame for the test frames.
This commit remove tests that only served a utility function rather then testing.
This commit renames StatelessVideoEncoder into VideoEncoder and moves it into the encoder module. This change is motivated that upcoming stateful encoder shares a lot of similarities and the same trait will be used.
This commit update drm dependency to fix compilation for other targets.
…size The field FrameMetadata::display_resolution was redundant, therefore removing it.
Some parsers use 0x00000001 byte sequence for NALu slicing instead of 0x000001, causing parsing failures. This change prepends 0x00 byte before slice NALu in order to prevent those failures.
This change enables direct use of DmabufFrame and UserPtrFrame directly with Vaapi encoders.
We need this library for `fstat` in order to acquire dmabuf size. It will be used for fixing encoding failure on Intel MTL devices.
This change fixes VA_STATUS_ERROR_ENCODING_ERROR status when DmabufFrame is used for encoding. It is fixed by providing fd/plane size information to libva.
This change add ability to allocate UserPtrFrame using provided FrameLayout for finer allocations for platform specific needs.
This commit moves codec structs and their EncoderConfig from src/encoder/stateless up to src/encoder. This is purely cosmetic change now, but since those structs are used for both stateless and future stateful implementation, thus should reside in common modules. In the future it will enable disable some implementations on the compile time.
This change just moves PredictionStructure enum definition to encoder module. This will enable conditional compilation of stateful and stateless modules in the future.
This commit adds v4l2 feature which will be used for V4L2 based codecs. The feature depends on v4l2r library, which was included in the Cargo.toml as well. The Cargo.lock was updated accordingly.
This commit adds V4L2 stateful encoder implementation using v4l2r. Currently only H.264 encoding is implemented.
This commit adds VP9 stateful encoder with additional necessary v4l2 controls.
This commit adds H.265 stateful encoder with additional necessary v4l2 controls.
This commit adds VP8 stateful encoder with additional necessary v4l2 controls.
Merge after a few final tweaks, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good - thank you!
Thank you! |
This PR is based on top of #73.