When decoding H.264, ffmpeg tries to set the frame format before
receiving the first DRC event, which is currently rejected even though
this is valid from the point of view of the client. We need to silently
accept the temporary format and all queued output buffers until the
initial DRC event is fired, even though we won't emit any frame.
BUG=None
TEST=`ffmpeg -codec:v h264_v4l2m2m -i Big_Buck_Bunny_720_10s_1MB.mp4 BBB-%d.png`
completes successfully in the guest and produces valid frames.
Change-Id: I9a190ab2748bd59ff57d54be681f2a1553d1c6a3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3841484
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Fix, extend, and reformat the documentation of libva to be closer to the
style of the standard library.
BUG=b:214478588
TEST=`cargo doc --document-private-items` in `media/libva` does not
generate any error.
Change-Id: Iee18f3471ddf8f2e65b5111f40fad3a020eafe41
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3932442
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
libva Displays and Contexts are only useful if they are
reference-counted, and all callers of `create_context` immediately wraps
the result into a Rc. Make `create_context` return a `Rc` directly to
make sure we cannot end up with stray objects.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=cargo test --features "video-decoder,vaapi" -p devices
Change-Id: I04c90059df71ea8a09a1fa937d731e1f4a5c27fc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3932441
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
The attributes array can be passed as empty to indicate we don't have
any preference for the configuration (which is what passing None
results into anyway), so remove the Option in order to simplify the
arguments.
BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=cargo test --features "video-decoder,vaapi" -p devices
Change-Id: I677b61f595f82cfbb519da3d78f433478eede8b9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3932439
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
The raw capabilities should not depend on the value for rt_format.
Both in the Intel and Mesa implementations it can be seen that setting
VAConfigAttribRTFormat as a parameter to vaCreateConfig only serves as a
way to validate that the to-be-generated config supports it.
This means that the Intel implementation will only use it in
CheckAttribList, while Mesa will AND it with a precomputed bitmask of
supported values in vaCreateConfig.
Thus, creating one RawCaps instance per value of RT_FORMAT only serves
to create needless duplication.
Not only that, but it frequently makes the call to QUERY_CAPABILITIES
fail, as the resulting structures become too large to fit into the 1024
byte buffer.
Remove all traces of VA_RT_FORMAT_* from this code path. While we are at
it, note that the libva crate API for vaQuerySurfaceAttribute changed to
reflect that this call can return more than one attribute at once.
This is particularly important for VASurfaceAttribPixelFormat, as one
value per supported fourcc is returned. We now properly return one
instance of RawCaps per VA_FOURCC supported.
Also reflect that Surface attributes can be defined more than once
This is particularly the case with VASurfaceAttribPixelFormat, where the
driver will define one attrib per pixel format supported.
This can be seen in the Intel driver, for example, in
MediaLibvaCaps::QuerySurfaceAttributes.
The previous code assumed that a given attribute could only be defined
once, and thus used find(). It was refactored to use filter() instead.
The name of the method was changed to reflect that it can now return
more than one attribute per call. The return type was changed to Vec for
the same reason.
BUG=b:214478588
TEST="cargo test --package devices --lib --features video-decoder --features vaapi -- virtio::video::decoder::backend::vaapi::tests::test_get_capabilities --include-ignored"
Change-Id: I914376beb026e30b2a52ca8f7d01c81e6a9a2775
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3946301
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
It's not ergonomic for the calling code to pass in an array of
references.
Just take ownership of that array, as it's not going to be used again
when the calling code succeeds in creating the wrapper.
BUG=b:214478588
TEST="cargo build --features=video-decoder,vaapi" successfully builds
the libva crate
Change-Id: I97d91f9f996c298450e65c21c126a9739ec4615f
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3900314
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Implement access to vaQuerySurfaceStatus for Picture<PictureEnd>. This
means that a decoder can initiate a decode operation and query its
status at a convenient time instead of blocking on it for completion.
Also expose libva::VASurfaceStatus to calling code while at it, since it
is needed to interface with this API.
BUG=b:214478588
TEST="cargo build --features=video-decoder,vaapi" successfully builds
the libva crate
Change-Id: I8a20455ef9937a4a54943325d359fa361190d8a4
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3875042
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Previously, only PictureSync would allow us to retrieve a borrow to the
underlying surface, but this is too restrictive. If we can take the
Surface back by calling take_surface() in PictureReclaimableSurface,
there is no reason not to be able to get a borrow to a Surface as well.
This makes the calling code more ergonomic.
BUG=b:214478588
TEST="cargo build --features=video-decoder,vaapi" successfully builds
the libva crate
Change-Id: I71f9f46e6bac6188c4c3862332c445c68028dbef
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3783006
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
In preparation for interlaced support, allow for picture creation using
another picture's Surface. This is so one can decode both fields to the
same underlying surface.
BUG=b:214478588
TEST="cargo build --features=video-decoder,vaapi" successfully builds
the libva crate
Change-Id: If3ffcc2592b7213644ed92f7202502fc92bac4b3
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3783005
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
While the current design works well for progressive content, in
interlaced content a common strategy is to decode both fields to the
same Surface.
Thus, add support for sharing the underlying Surface. This means that we
can only retrieve the Surface if we are the last Picture with a
reference to it.
While we are at it: only retrieve Surfaces if we are the last users
In preparation for interlaced decoding support, only retrieve Surfaces
back into the surface pool if we are the last users. This means that the
surface will not be dropped until both of its fields have been dropped.
BUG=b:214478588
TEST="cargo build --features=video-decoder,vaapi" successfully
builds the libva crate
Change-Id: I80412a86f51ab639fab6af229572fb7bc800a928
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3946300
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
It's not ergonomic for the calling code to pass in an array of
references.
Just take ownership of that array, as it's not going to be used again
when the calling code succeeds in creating the wrapper.
BUG=b:214478588
TEST="cargo test"
Change-Id: Ifbb78968ebb29b41fe2ccaa653f30a81fcb23427
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3783002
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
This requires a few tweaks to non-generated code:
- VIRTIO_ID_VIDEO_ENC/DEC -> ENCODER/DECODER
- io_uring unnamed union layout change
Change-Id: I58e118efa5c6bf28ff56d211fec5603651cb60bc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3893753
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
While we are tweaking all of the copyright headers, let's take the
opportunity to ensure there is always a blank line after the copyright
header for consistency. (Almost all files already follow this style.)
This includes a slightly ugly regex to allow the end of a C-style
comment block after the end of the copyright:
/*
* Example comment block
*/ <-- this line
Change-Id: Idfd0855861e5ecb3d33afae942fdba908af0dcff
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3892521
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
This search/replace updates all copyright notices to drop the
"All rights reserved", Use "ChromiumOS" instead of "Chromium OS"
and drops the trailing dots.
This fulfills the request from legal and unifies our notices.
./tools/health-check has been updated to only accept this style.
BUG=b:246579983
TEST=./tools/health-check
Change-Id: I87a80701dc651f1baf4820e5cc42469d7c5f5bf7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3894243
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
File::open() opens the file as O_RDONLY, which can cause issues with the
VA-API driver.
In particular for Intel hardware this will cause allocations to fail, as can
be seen in issue #1449 for intel-media-driver. These failed allocations may or
may not crash the VA-API driver, as the driver might eventually dereference a
NULL pointer.
Fix it by opening the DRM fd as O_RDWR. This is also in line with the examples
in libva-utils.
BUG=b:214478588
TEST=`cargo test --features "video-decoder,vaapi" -p devices vaapi::tests::test_get_capabilities -- --ignored` passes on AMD hardware.
Change-Id: Ie3cf2a6512157a3f23f943b54249eb2928082af9
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3782999
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Tested-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Fix the broken timestamp management in the virtio decoder device.
Timestamps are specified by the guest's user-space as a struct timeval,
which is effectively a (seconds, microseconds) pair. This pair is turned
into a u64 with nanoseconds units by V4L2, and passed to the virtio
device in that form.
This is fine so far, save for the fact that libvda works with 32-bit
timestamps. To accomodate that, we divided the original timestamp by
1_000_000_000, passed that truncated timestamp to the decoder backend as
a u32, and when the backend gives us decoded frames, multiplied their
timestamp by 1_000_000_000 again and passed that value as the frame
timestamp to the guest. For some reason, we also used the timestamp as
the key for the `NotifyEndOfBistreamBuffer` event, which forced us to
have a mapping table between an input buffer timestamp and its
corresponding resource ID. This in turn required that each input buffer
has a unique timestamp, lest they collide in the mapping table.
Anyway, the timestamp division by 1_000_000_000 means that any
sub-second timestamp information was lost during the decoding process.
This is not a problem with the Android V4L2 decoder which increases the
timestamp by one second for each frame and does not use sub-second
information, but clients that use *actual* timestamp information (like
ffmpeg) get pretty confused by that and abandon decoding with an error.
Fix this by passing the original 64-bit timestamp to decoder backends,
along with the 32-bit ID of the input resource. This allows backends to
directly send the NotifyEndOfBitstreamBuffer with the currect resource
ID, removing the need for the mapping table and unique timestamps for
input buffers.
More importantly, backends that can work with 64-bit timestamps (all of
them but libvda) can just pass the original timestamp to their decoder
logic and return it as-is in the PictureReady event. Libvda still
requires 32-bit timestamps, so we reproduce the truncation and mapping
table behavior inside the libvda backend (I also tried sending the
32-bit input resource ID as a timestamp to VDA, but it was not happy
with it). At least now that behavior is local to the VDA backend.
Thanks to this change clients other than the Android C2 decoder can get
proper timestamp information. One can argue that the code is also
simpler (except for libvda which adds a simple mapping table that we
were using before anyway).
BUG=b:161774071
TEST=cargo test --features "video-decoder,ffmpeg" -p devices video
TEST=`ffmpeg -codec:v vp8_v4l2m2m -i test-25fps.vp8 test-25fps-%d.png`
in the guest completes successfully with the ffmpeg backend.
TEST=tast arc.VideoDecodeAccel*_vm passes on hatch.
Change-Id: Idb21b4c536acafbdf5458e88cdbc33c9376a405e
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3841480
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Tested-by: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Tatsuyuki Ishi <ishitatsuyuki@google.com>
crosvm is switching the import style to use one import per line.
While more verbose, this will greatly reduce the occurence of merge
conflicts going forward.
Note: This is using a nightly feature of rustfmt. So it's a one-off
re-format only. We are considering adding a nightly toolchain to
enable the feature permanently.
BUG=b:239937122
TEST=CQ
Change-Id: Id2dd4dbdc0adfc4f8f3dd1d09da1daafa2a39992
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3784345
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Tested-by: Dennis Kempin <denniskempin@google.com>
Commit-Queue: Dennis Kempin <denniskempin@google.com>
Fixes a Rust 1.60 clippy lint:
error: writing `&mut Vec` instead of `&mut [_]` involves a new
object where a slice will do
BUG=b:239075544
TEST=tools/clippy # with rust-toolchain = 1.60.0
Change-Id: I273bd647067b807982f9a9b7248ba788ad3f2032
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3764421
Tested-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
This is a reland of commit 213f9fe8a7.
In light of the upcoming VAAPI video decoder backend, add a VAAPI
wrapper crate that exposes a safe Rust API for a subset of the VAAPI C
code. This crate will be called from the VAAPI video decoder backend in
order to decode frames.
BUG=b:214478588
TEST=cargo build --features "video-decoder,vaapi"
TEST=`cargo test -- --include-ignored` in `media/libva` passes on a
device with Intel GPU and libva installed.
Change-Id: I586a160e477e466985c5cfa65a527542ddc03226
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3752274
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
This reverts commit 213f9fe8a7.
Reason for revert: breaks the ChromeOS builder.
Original change's description:
> Add a VAAPI wrapper crate
>
> In light of the upcoming VAAPI video decoder backend, add a VAAPI
> wrapper crate that exposes a safe Rust API for a subset of the VAAPI C
> code. This crate will be called from the VAAPI video decoder backend in
> order to decode frames.
>
> BUG=b:214478588
> TEST=cargo build --features "video-decoder,vaapi"
> TEST=`cargo test -- --include-ignored` in `media/libva` passes on a
> device with Intel GPU and libva installed.
>
> Change-Id: I4afa96c49d045251827b97bd78faeba57575aedc
Bug: b:214478588
Change-Id: Ib5a88cd4c5fdd2df2e69fd3a0896ee789585840d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3752267
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
In light of the upcoming VAAPI video decoder backend, add a VAAPI
wrapper crate that exposes a safe Rust API for a subset of the VAAPI C
code. This crate will be called from the VAAPI video decoder backend in
order to decode frames.
BUG=b:214478588
TEST=cargo build --features "video-decoder,vaapi"
TEST=`cargo test -- --include-ignored` in `media/libva` passes on a
device with Intel GPU and libva installed.
Change-Id: I4afa96c49d045251827b97bd78faeba57575aedc
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3422779
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
