From 4ffb3d06bdadcfcb79ee2b7bf445ac09da34c218 Mon Sep 17 00:00:00 2001 From: Keiichi Watanabe Date: Wed, 13 May 2020 19:43:02 +0900 Subject: devices: video: dec: Support arbitrary buffers to be mapped as resources Support a case where a guest client who may use arbitrary numbers of buffers. (e.g. C2V4L2Component with default pool in ARCVM) Such a client is valid as long as it uses at most 32 buffers at the same time. More specifically, this CL allows the guest to call ResourceCreate for an output resource_id which was already processed by the host. Such ResourceCreate calls will be handled as reassignment of DMAbuf to a FrameBufferId. BUG=b:157702336 TEST=Play a YouTube video on ARCVM w/ C2V4L2Component using default pool Change-Id: Ie9c457867abd91b6b7a17a5bca4a1a1e9f53c1ae Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2198327 Reviewed-by: Alexandre Courbot Tested-by: Keiichi Watanabe Commit-Queue: Keiichi Watanabe --- devices/src/virtio/video/decoder/mod.rs | 85 +++++++++++++++++++++++---------- 1 file changed, 59 insertions(+), 26 deletions(-) (limited to 'devices/src/virtio/video/decoder/mod.rs') diff --git a/devices/src/virtio/video/decoder/mod.rs b/devices/src/virtio/video/decoder/mod.rs index 3516cad..1dfd35c 100644 --- a/devices/src/virtio/video/decoder/mod.rs +++ b/devices/src/virtio/video/decoder/mod.rs @@ -5,7 +5,7 @@ //! Implementation of a virtio video decoder device backed by LibVDA. use std::collections::btree_map::Entry; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::convert::TryInto; use std::fs::File; use std::os::unix::io::{AsRawFd, IntoRawFd}; @@ -42,6 +42,8 @@ type FrameBufferId = i32; type ResourceHandle = u32; type Timestamp = u64; +const OUTPUT_BUFFER_COUNT: usize = 32; + // Represents queue types of pending Clear commands if exist. #[derive(Default)] struct PendingClearCmds { @@ -64,9 +66,14 @@ struct Context { res_id_to_res_handle: BTreeMap, // OutputResourceId <-> FrameBufferId + // TODO: Encapsulate the following two maps as a type of a bijective map. res_id_to_frame_buf_id: BTreeMap, frame_buf_id_to_res_id: BTreeMap, + // Stores free FrameBufferIds which `use_output_buffer()` or `reuse_output_buffer()` can be + // called with. + available_frame_buf_ids: BTreeSet, + keep_resources: Vec, // Stores queue types of pending Clear commands if exist. @@ -127,12 +134,31 @@ impl Context { self.res_id_to_res_handle.insert(resource_id, handle); } - fn register_queued_frame_buffer(&mut self, resource_id: OutputResourceId) -> FrameBufferId { - // Generate a new FrameBufferId - let id = self.res_id_to_frame_buf_id.len() as FrameBufferId; + fn register_queued_frame_buffer( + &mut self, + resource_id: OutputResourceId, + ) -> VideoResult { + // Use the first free frame buffer id. + let id = *self.available_frame_buf_ids.iter().next().ok_or_else(|| { + error!( + "no frame buffer ID is available for resource_id {}", + resource_id + ); + VideoError::InvalidOperation + })?; + self.available_frame_buf_ids.remove(&id); + + // Invalidate old entries for `resource_id` and `id` from two maps to keep them bijective. + if let Some(old_frame_buf_id) = self.res_id_to_frame_buf_id.remove(&resource_id) { + self.frame_buf_id_to_res_id.remove(&old_frame_buf_id); + } + if let Some(old_res_id) = self.frame_buf_id_to_res_id.remove(&id) { + self.res_id_to_frame_buf_id.remove(&old_res_id); + } + self.res_id_to_frame_buf_id.insert(resource_id, id); self.frame_buf_id_to_res_id.insert(id, resource_id); - id + Ok(id) } fn reset(&mut self) { @@ -200,6 +226,9 @@ impl Context { right: i32, bottom: i32, ) -> Option { + // `buffer_id` becomes available for another frame. + self.available_frame_buf_ids.insert(buffer_id); + let plane_size = ((right - left) * (bottom - top)) as u32; for fmt in self.out_params.plane_formats.iter_mut() { fmt.plane_size = plane_size; @@ -239,6 +268,10 @@ impl Context { } else if self.pending_clear_cmds.output { self.pending_clear_cmds.output = false; + // Reinitialize `available_frame_buf_ids`. + self.available_frame_buf_ids.clear(); + self.available_frame_buf_ids = (0..OUTPUT_BUFFER_COUNT as i32).collect(); + Some(QueueType::Output) } else { error!("unexpected ResetResponse"); @@ -426,8 +459,6 @@ impl<'a> Decoder<'a> { // first time. let mut ctx = self.contexts.get_mut(&stream_id)?; if !ctx.set_output_buffer_count { - const OUTPUT_BUFFER_COUNT: usize = 32; - // Set the buffer count to the maximum value. // TODO(b/1518105): This is a hack due to the lack of way of telling a number of // frame buffers explictly in virtio-video v3 RFC. Once we have the way, @@ -436,20 +467,18 @@ impl<'a> Decoder<'a> { .get(&stream_id)? .set_output_buffer_count(OUTPUT_BUFFER_COUNT) .map_err(VideoError::VdaError)?; + + // FrameBufferId must be in the range of 0.. ((# of buffers) - 1). + ctx.available_frame_buf_ids = (0..OUTPUT_BUFFER_COUNT as i32).collect(); + ctx.set_output_buffer_count = true; } - // We assume ResourceCreate is not called to an output resource that is already - // imported to Chrome for now. - // TODO(keiichiw): We need to support this case for a guest client who may use - // arbitrary numbers of buffers. (e.g. C2V4L2Component in ARCVM) - // Such a client is valid as long as it uses at most 32 buffers at the same time. + // If the given resource_id was associated with an old frame_buf_id, + // dissociate them. if let Some(frame_buf_id) = ctx.res_id_to_frame_buf_id.get(&resource_id) { - error!( - "resource {} has already been imported to Chrome as a frame buffer {}", - resource_id, frame_buf_id - ); - return Err(VideoError::InvalidOperation); + ctx.frame_buf_id_to_res_id.remove(&frame_buf_id); + ctx.res_id_to_frame_buf_id.remove(&resource_id); } Ok(()) @@ -545,14 +574,18 @@ impl<'a> Decoder<'a> { // In case a given resource has been imported to VDA, call // `session.reuse_output_buffer()` and return a response. // Otherwise, `session.use_output_buffer()` will be called below. - match ctx.res_id_to_frame_buf_id.get(&resource_id) { - Some(buffer_id) => { - session - .reuse_output_buffer(*buffer_id) - .map_err(VideoError::VdaError)?; - return Ok(()); - } - None => (), + if let Some(buffer_id) = ctx.res_id_to_frame_buf_id.get(&resource_id) { + if !ctx.available_frame_buf_ids.remove(&buffer_id) { + error!( + "resource_id {} is associated with VDA's frame_buffer id {}, which is in use or out of range.", + resource_id, buffer_id + ); + return Err(VideoError::InvalidOperation); + } + session + .reuse_output_buffer(*buffer_id) + .map_err(VideoError::VdaError)?; + return Ok(()); }; let resource_info = ctx.get_resource_info(resource_bridge, resource_id)?; @@ -579,7 +612,7 @@ impl<'a> Decoder<'a> { let buffer_id = self .contexts .get_mut(&stream_id)? - .register_queued_frame_buffer(resource_id); + .register_queued_frame_buffer(resource_id)?; session .use_output_buffer(buffer_id as i32, libvda::PixelFormat::NV12, fd, &planes) -- cgit 1.4.1