Commit graph

27 commits

Author SHA1 Message Date
Alexandre Courbot
a074f28164 media: cros-codecs: vaapi: remove AssociatedBackendHandle types
We don't really need to use the BackendHandle associated type here as we
know that vaapi will always use the GenericBackendHandle - so let's just
use this and simplify the type map a bit.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: Idb77ecda7bdd566ffb7bc46a35ebaadcb11cdeef
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4060493
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-11-30 01:36:07 +00:00
Alexandre Courbot
feba776773 media: cros-codecs: factorize dummy backends
The dummy backends behave the same regardless of the codec, so factorize
their declaration as well as their implementation of VideoDecoderBackend
to avoid duplicate code.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: Ibaa694b885c1ccf969aed3a0cb167a254e0eaab5
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4060492
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-11-30 01:21:49 +00:00
Clarissa Garvey
0292091f52 Fix needless return violations
...round 2  because building locally missed some :)

Most are safe, similar to crrev.com/c/4014832

Two require an exception, similar to crrev.com/c/4018392

BUG=b:157245930, crbug:908640
TEST=CQ with needless_return suppression disabled

Change-Id: I0f54ee1e4688afe9c9f596bd32a98070aac20612
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4019273
Commit-Queue: Clarissa Garvey <clarissagarvey@chromium.org>
Reviewed-by: Dennis Kempin <denniskempin@google.com>
2022-11-10 18:10:31 +00:00
Alexandre Courbot
2824a8e163 media: cros-codecs: factorize DecodedHandle implementations
The DecodedHandle implementations are almost identical across codecs,
save for that display_resolution() method. Abstract it into the
codec-specific data of the picture using a new FrameInfo trait, so that
each backend can share a single implementation of DecodedHandle. This
also allows us to factorize the dummy Handle structs into a single
generic struct.

BUG=b:214478588
TEST=cargo build --features `video-decoder,video-encoder,ffmpeg,vaapi`
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: I959ca39e79f8f7c81d1c75ab42dc4f0b4c67262b
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4012399
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-11-10 09:11:49 +00:00
Alexandre Courbot
e516e84013 media: cros-codecs: implement MappableHandle for Image
MappableHandle was implemented on the backend handle, but doing so is
sub-optimal in the case of VAAPI which needs to re-create the VA Image
with each call to a method of MappableHandle, which is a costly
operation.

Fix this by implementing MappableHandle on the Image directly. That way
callers can request the mappable handler once to create the image, and
call all the methods on the same instance.

This also allows us to make MappableHandle::image_size return the size
instead of a Result.

BUG=b:214478588
TEST=cargo build --features `video-decoder,video-encoder,ffmpeg,vaapi`
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: Iaa048c1f488021d49376f612ebde560e84a11dc4
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4006228
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2022-11-10 09:11:49 +00:00
Alexandre Courbot
1533d1455b media: cros-codecs: factorize dummy backend handles
The dummy BackendHandles used for tests are the same across all codecs,
so share them.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: Ic9bd84c24363f9fbb255480ab2817c7c16dcebdc
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4006227
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-11-10 09:11:49 +00:00
Alexandre Courbot
5b3bfeb237 media: cros-codecs: implement methods for GenericBackendHandle
GenericBackendHandle is created, accessed and modified by the various
decoders directly, but the access patterns could easily be fulfilled by
a few methods.

This also allows us to make GenericBackendHandleInner private, as its
name suggests it should be, and to simplify it a bit.

BUG=b:214478588
TEST=cargo build --features `video-decoder,video-encoder,ffmpeg,vaapi`
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: I9ef2a58361db37b69315c91e9ad337fd0c99b8cb
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005493
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>
2022-11-10 09:11:49 +00:00
Alexandre Courbot
49b08a0e2c media: cros-codecs: replace mapped resolution with image size in bytes
The concept of mapped resolution is only used in the context of copying
the frame to a CPU-mapped buffer in order to compute the size of said
buffer according to the underlying format.

This requires the caller to check the buffer format and perform the
computation themselves. It would be more efficient to just have a
function performing that computation in a single place and returning the
final buffer size.

Thus, replace MappedHandle's mapped_resolution() method with
image_size(), which returns the size of the buffer that should be passed
to read().

BUG=b:214478588
TEST=cargo build --features `video-decoder,video-encoder,ffmpeg,vaapi`
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: Icff17aa5e2e08676edb450a53316e44897604958
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005492
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
2022-11-10 09:11:49 +00:00
Alexandre Courbot
d80d2e7176 media: cros-codecs: remove MappableHandle::map
We never use this method, so remove it. Besides how to copy an image is
dependent of the format and handled by read(), so this should be the
prefered way of accessing frames.

This also allows us to remove the dummy mapped handles.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: I4909a33f2ba3dbcc61e27493ed0ad3b8a75cd882
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005491
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
2022-11-09 02:45:32 +00:00
Alexandre Courbot
bf65947d00 media: cros-codecs: remove unused SurfacePoolHandle methods
These methods are unused in the code.

BUG=b:214478588
TEST=cargo build --features `video-decoder,video-encoder,ffmpeg,vaapi`
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: I275d0c26b70922b13fd7ec1a133f20de60987de0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005490
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
2022-11-09 02:39:04 +00:00
Alexandre Courbot
97fe4ec8e8 media: cros-codecs: harmonize error descriptions
Error descriptions should have no leading capital and do not end with a
dot.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: I807e4874c862bc8e32dc4a394b1e5e46df60c8d0
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005489
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>
2022-11-09 02:35:58 +00:00
Alexandre Courbot
13f21379ac media: cros-codecs: remove unneeded type suffixes
Indices are already usize implicitly.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: I555a4feb5669906c73004e598744311e3351090a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005488
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>
2022-11-09 02:30:13 +00:00
Alexandre Courbot
9add805a30 media: cros-codecs: remove InnerHandle types
These types are only used in two places, and their actual definition is
more explicit about their purpose.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.

Change-Id: I90eec9751bc4aaccab33fe70d96a94a5875f6f6c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3995769
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2022-11-09 02:27:24 +00:00
Alexandre Courbot
54ce45da43 media: cros-codecs: factorize DynDecodedHandle definitions
The DynDecodedHandle traits defined for each codec are basically
identical, and can be simplified into a single one now that we also have
a common DecodedHandle struct.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.

Change-Id: I666d941dfea5b5e40ca46dab661e87aec60d65e4
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998376
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>
2022-11-09 02:21:42 +00:00
Alexandre Courbot
5e6927adeb media: cros-codecs: factorize DecodedHandle definitions
The DecodedHandle structs defined for each codec are basically identical
and can be simplified into a single one by adding an associated type.

Doing so required the H.264 types to be constrained with non-trivial
where clauses due to the fact the H264 PictureData is itself generic,
but there is a plan to simplify this too.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.

Change-Id: Idcb0717740e6e700ae83d758f99ba0035981dcca
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998375
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>
2022-11-09 01:47:41 +00:00
Alexandre Courbot
0545497647 media: cros-codecs: factorize SurfaceContainer implementations
SurfaceContainer was implemented for each codec type, but it can be
simplified into a single generic implementation if move the RefCell out
of InnerHandle.

Since SurfaceContainer's purpose is just to convert one object into a
Surface, rename it into the more idiomatic IntoSurface that just
requires TryInto<Option<Surface>>.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.

Change-Id: I00a56132ceaeab5d305e144976a9ea597355bbf8
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998374
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2022-11-09 01:47:41 +00:00
Alexandre Courbot
b8c4b8289c media: cros-codecs: factorize Picture definitions
The Picture structs defined for each codec are now basically identical,
and can be simplified into a single one by adding one generic argument.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs:wq
TEST=`cros-codecs --include-ignored` passes on hatch.

Change-Id: I48a79c615f7c53de5d39bcf37da0b64d05c65a09
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998373
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.corp-partner.google.com>
2022-11-09 01:47:41 +00:00
Alexandre Courbot
1c962fc899 media: cros-codecs: move H.264 picture data into its own struct
The Picture structs defined for each codec are essentially identical,
with only the H.264 one having its codec-specific data inlined into the
struct instead of being part of another one. Move the H.264 data in
order to make the 3 structures similar. This will allow us to merge them
into one common struct in the next CL.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.

Change-Id: I3282fde1dbbc2ad303f2691e00c46aa8b6c0de41
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998372
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>
2022-11-09 01:47:41 +00:00
Alexandre Courbot
acc3aa6532 media: cros-codecs: simplify the Picture interface
backend_handle_unchecked* were only used in tests, and the DynPicture
interface provides equivalent functions anyway, so use these ones and
drop the former.

Ideally these methods that can potentially panic should be replaced by
something else, but this will have to wait further cleanups.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs
TEST=`cros-codecs --include-ignored` passes on hatch.

Change-Id: I339f6893599c5ffc24e738f5ad11f35cbfcaab25
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3998371
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-11-09 01:47:41 +00:00
Alexandre Courbot
9206613bdc media: cros-codecs: do not export module local types
These types do not need to be exported.

BUG=b:214478588
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: I5310bb4ce34956d6f6edbbe8a8852b4c18b8130c
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4005487
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2022-11-07 10:58:03 +00:00
Alexandre Courbot
02eff72038 media: cros-codecs: make poll() take BlockingMode directly
The `blocking` boolean was converted into a `BlockingMode` anyway, and
using `BlockingMode` directly is also more explicit about what the
parameters is about.

BUG=b:214478588
TEST=cargo build --features `video-decoder,video-encoder,ffmpeg,vaapi`
TEST=cargo test --features vaapi -p cros-codecs

Change-Id: If72067d8218d53ff8d092cef384909d282862e0a
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3995885
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Auto-Submit: Alexandre Courbot <acourbot@chromium.org>
2022-11-07 10:54:58 +00:00
Daniel Verkamp
4eed9244b5 cros-codecs: minor cargo doc cleanups
Use backticks around comments that use array syntax with [] to avoid
interpreting those as Markdown links.

Additionally, convert a few file headers to //! so they show up in the
generated docs.

BUG=None
TEST=tools/cargo-doc

Change-Id: I34799ff8e0e48b799f31255ae1ed84a0c18e8601
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3993931
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Auto-Submit: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
2022-11-01 07:52:53 +00:00
Alexandre Courbot
5f01cee736 media: libva: create Rc'd displays and contexts
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>
2022-10-31 10:35:23 +00:00
Alexandre Courbot
0ae64281f3 media: libva: remove redundant Option for create_config
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>
2022-10-31 10:35:23 +00:00
Daniel Almeida
af92d04e75 media: cros-codecs: decoders: allow for polling the decoders
Allow for polling the decoders. This will return any pending decode
requests that happen to be ready. If block is set to true, it will
block until they are all ready.

This is different from flush, in that flush will empty the decoder's
internal state, altering the DPB, whereas poll will not.

BUG=b:214478588
TEST=cd media/cros-codecs && cargo test --features vaapi

Change-Id: I56b1b9d7b704c17d75bbe93d4fd4eb0a9a468af7
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3918838
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-10-28 07:41:58 +00:00
Daniel Almeida
d53c442b8e media: cros-codecs: make it possible to retrieve the display order
Make it possible to retrieve the display order by adding a
display_order() member to the DynDecodedHandle trait.

This is in preparation for further CrosVM changes where the handles
will be sorted by the display order.

The decoders will issue the display order from a monotonically
increasing counter after applying any codec-specific reordering
algorithm. We will build upon the monotonically increasing counter to
identify gaps in CrosVM and wait until said gaps are filled by newer
frames.

BUG=b:214478588
TEST="cargo build --features=video-decoder,vaapi" successfully builds
the libva crate

Change-Id: I4e4b1becb9aea124b6b942069dcc676385bd6986
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3907528
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
2022-10-28 07:36:23 +00:00
Daniel Almeida
42bdf1de57 media: cros-codecs: Introduce the cros-codecs crate
Introduce the cros-codecs crate. This crate contains all the
codec-related code and does not depend on CrosVM. The decoders are
decoupled from the backends, which allows for the implementation of new
backends without touching the decoder code.

This crate comes with dummy backends to test the decoder functionality
in isolation, but in order to decode frames, a real backend is needed.
Currently this backend is the VAAPI backend. Using it adds a dependency
on the libva crate.

This change adds support for VP8, H264 and VP9.

BUG=b:214478588
TEST="cd media/cros-codecs && cargo test --release --features vaapi -- --include-ignored"
TEST="emerge-hatch chromeos-base/crosvm" completes successfully.

Change-Id: I596d5db4dabcc96dcfdbce1f41c8092e01b64271
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3875043
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
2022-10-28 03:23:26 +00:00