virtio: video: Enrich GuestResource with more metadata.

This adds:
- A buffer size field for each plane
- Width, height, format field for each image

This is for introducing helpers to convert video GuestResources into
AvFrames.

In practice, these metadata were readily available either from the
protocol or through rubataga RPCs, but they have not been stored in the
GuestResource struct. With these change these fields would be available
for usage in constructing AvFrames.

A tricky topic is the source of the metadata. Right now, neither the
virtio_video_params struct nor virtio-gpu backend metadata alone can
provide all the metadata we need. See code comment in
GuestResource::from_virtio_object_entry and go/crosvm-enc-ffmpeg for
future work discussions.

BUG=b:239897269
TEST=cargo test --features "video-decoder,ffmpeg" -p ffmpeg -p devices

Change-Id: Icb47274d2dd379a10bdf878ed91ffbc238685c6d
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3911110
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>

devices/src/virtio/video/decoder/backend/ffmpeg.rs

@@ -990,8 +990,10 @@ mod tests {
                     }
                 ));
 
+                let out_format = Format::NV12;
+
                 session
-                    .set_output_parameters(NUM_OUTPUT_BUFFERS, Format::NV12)
+                    .set_output_parameters(NUM_OUTPUT_BUFFERS, out_format)
                     .unwrap();
 
                 // Pass the buffers we will decode into.
@@ -1005,12 +1007,17 @@ mod tests {
                                     FramePlane {
                                         offset: 0,
                                         stride: H264_STREAM_WIDTH as usize,
+                                        size: (H264_STREAM_WIDTH * H264_STREAM_HEIGHT) as usize,
                                     },
                                     FramePlane {
                                         offset: (H264_STREAM_WIDTH * H264_STREAM_HEIGHT) as usize,
                                         stride: H264_STREAM_WIDTH as usize,
+                                        size: (H264_STREAM_WIDTH * H264_STREAM_HEIGHT) as usize,
                                     },
                                 ],
+                                width: H264_STREAM_WIDTH as _,
+                                height: H264_STREAM_HEIGHT as _,
+                                format: out_format,
                             },
                         )
                         .unwrap();

devices/src/virtio/video/decoder/mod.rs

@@ -526,9 +526,9 @@ impl<D: DecoderBackend> Decoder<D> {
         };
 
         // Now try to resolve our resource.
-        let (resource_type, plane_formats) = match queue_type {
-            QueueType::Input => (ctx.in_params.resource_type, &ctx.in_params.plane_formats),
-            QueueType::Output => (ctx.out_params.resource_type, &ctx.out_params.plane_formats),
+        let (resource_type, params) = match queue_type {
+            QueueType::Input => (ctx.in_params.resource_type, &ctx.in_params),
+            QueueType::Output => (ctx.out_params.resource_type, &ctx.out_params),
         };
 
         let resource = match resource_type {
@@ -543,6 +543,7 @@ impl<D: DecoderBackend> Decoder<D> {
                     // exactly one element.
                     unsafe { entries.get(0).unwrap().object },
                     &self.resource_bridge,
+                    params,
                 )
                 .map_err(|_| VideoError::InvalidArgument)?
             }
@@ -555,7 +556,7 @@ impl<D: DecoderBackend> Decoder<D> {
                     )
                 },
                 &self.mem,
-                plane_formats,
+                params,
             )
             .map_err(|_| VideoError::InvalidArgument)?,
         };

devices/src/virtio/video/encoder/mod.rs

@@ -672,6 +672,7 @@ impl<T: Encoder> EncoderDevice<T> {
                             // exactly one element.
                             unsafe { entries.get(0).unwrap().object },
                             &self.resource_bridge,
+                            &stream.src_params,
                         )
                         .map_err(|_| VideoError::InvalidArgument)?
                     }
@@ -684,7 +685,7 @@ impl<T: Encoder> EncoderDevice<T> {
                             )
                         },
                         &self.mem,
-                        &stream.src_params.plane_formats,
+                        &stream.src_params,
                     )
                     .map_err(|_| VideoError::InvalidArgument)?,
                 };
@@ -719,6 +720,7 @@ impl<T: Encoder> EncoderDevice<T> {
                             // exactly one element.
                             unsafe { entries.get(0).unwrap().object },
                             &self.resource_bridge,
+                            &stream.dst_params,
                         )
                         .map_err(|_| VideoError::InvalidArgument)?
                     }
@@ -731,7 +733,7 @@ impl<T: Encoder> EncoderDevice<T> {
                             )
                         },
                         &self.mem,
-                        &stream.dst_params.plane_formats,
+                        &stream.dst_params,
                     )
                     .map_err(|_| VideoError::InvalidArgument)?,
                 };

devices/src/virtio/video/format.rs

@@ -306,4 +306,5 @@ pub struct Rect {
 pub struct FramePlane {
     pub offset: usize,
     pub stride: usize,
+    pub size: usize,
 }

devices/src/virtio/video/resource.rs

@@ -23,8 +23,9 @@ use crate::virtio::resource_bridge;
 use crate::virtio::resource_bridge::ResourceBridgeError;
 use crate::virtio::resource_bridge::ResourceInfo;
 use crate::virtio::resource_bridge::ResourceRequest;
+use crate::virtio::video::format::Format;
 use crate::virtio::video::format::FramePlane;
-use crate::virtio::video::format::PlaneFormat;
+use crate::virtio::video::params::Params;
 use crate::virtio::video::protocol::virtio_video_mem_entry;
 use crate::virtio::video::protocol::virtio_video_object_entry;
 
@@ -178,6 +179,9 @@ pub struct GuestResource {
     pub handle: GuestResourceHandle,
     /// Layout of color planes, if the resource will receive frames.
     pub planes: Vec<FramePlane>,
+    pub width: u32,
+    pub height: u32,
+    pub format: Format,
 }
 
 #[derive(Debug, ThisError)]
@@ -207,11 +211,14 @@ impl GuestResource {
     /// resource.
     ///
     /// Convert `mem_entry` into the guest memory resource it represents and resolve it through
-    /// `mem`. `planes_format` describes the format of the individual planes for the buffer.
+    /// `mem`.
+    /// Width, height and format is set from `params`.
+    ///
+    /// Panics if `params.format` is `None`.
     pub fn from_virtio_guest_mem_entry(
         mem_entries: &[virtio_video_mem_entry],
         mem: &GuestMemory,
-        planes_format: &[PlaneFormat],
+        params: &Params,
     ) -> Result<GuestResource, GuestMemResourceCreationError> {
         let region_desc = match mem_entries.first() {
             None => return Err(GuestMemResourceCreationError::NoEntriesProvided),
@@ -245,9 +252,15 @@ impl GuestResource {
             })
             .collect();
 
+        let handle = GuestResourceHandle::GuestPages(GuestMemHandle {
+            desc: region_desc,
+            mem_areas,
+        });
+
         // The plane information can be computed from the currently set format.
         let mut buffer_offset = 0;
-        let planes = planes_format
+        let planes = params
+            .plane_formats
             .iter()
             .map(|p| {
                 let plane_offset = buffer_offset;
@@ -256,16 +269,17 @@ impl GuestResource {
                 FramePlane {
                     offset: plane_offset as usize,
                     stride: p.stride as usize,
+                    size: p.plane_size as usize,
                 }
             })
             .collect();
 
         Ok(GuestResource {
-            handle: GuestResourceHandle::GuestPages(GuestMemHandle {
-                desc: region_desc,
-                mem_areas,
-            }),
+            handle,
             planes,
+            width: params.frame_width,
+            height: params.frame_height,
+            format: params.format.unwrap(),
         })
     }
 
@@ -277,6 +291,7 @@ impl GuestResource {
     pub fn from_virtio_object_entry(
         object: virtio_video_object_entry,
         res_bridge: &base::Tube,
+        params: &Params,
     ) -> Result<GuestResource, ObjectResourceCreationError> {
         // We trust that the caller has chosen the correct object type.
         let uuid = u128::from_be_bytes(object.uuid);
@@ -296,24 +311,45 @@ impl GuestResource {
             Err(e) => return Err(ObjectResourceCreationError::ResourceBridgeFailure(e)),
         };
 
+        let handle = GuestResourceHandle::VirtioObject(VirtioObjectHandle {
+            // Safe because `buffer_info.file` is a valid file descriptor and we are stealing
+            // it.
+            desc: unsafe {
+                SafeDescriptor::from_raw_descriptor(buffer_info.handle.into_raw_descriptor())
+            },
+            modifier: buffer_info.modifier,
+        });
+
+        // TODO(ishitatsuyuki): Right now, there are two sources of metadata: through the
+        //                      virtio_video_params fields, or through the buffer metadata provided
+        //                      by the VirtioObject backend.
+        //                      Unfortunately neither is sufficient. The virtio_video_params struct
+        //                      lacks the plane offset, while some virtio-gpu backend doesn't
+        //                      have information about the plane size, or in some cases even the
+        //                      overall frame width and height.
+        //                      We will mix-and-match metadata from the more reliable data source
+        //                      below; ideally this should be fixed to use single source of truth.
+        let planes = params
+            .plane_formats
+            .iter()
+            .zip(&buffer_info.planes)
+            .map(|(param, buffer)| FramePlane {
+                // When the virtio object backend was implemented, the buffer and stride was sourced
+                // from the object backend's metadata (`buffer`). To lean on the safe side, we'll
+                // keep using data from `buffer`, even in case of stride it's also provided by
+                // `param`.
+                offset: buffer.offset as usize,
+                stride: buffer.stride as usize,
+                size: param.plane_size as usize,
+            })
+            .collect();
+
         Ok(GuestResource {
-            handle: GuestResourceHandle::VirtioObject(VirtioObjectHandle {
-                // Safe because `buffer_info.file` is a valid file descriptor and we are stealing
-                // it.
-                desc: unsafe {
-                    SafeDescriptor::from_raw_descriptor(buffer_info.handle.into_raw_descriptor())
-                },
-                modifier: buffer_info.modifier,
-            }),
-            planes: buffer_info
-                .planes
-                .iter()
-                .take_while(|p| p.offset != 0 || p.stride != 0)
-                .map(|p| FramePlane {
-                    offset: p.offset as usize,
-                    stride: p.stride as usize,
-                })
-                .collect(),
+            handle,
+            planes,
+            width: params.frame_width,
+            height: params.frame_height,
+            format: params.format.unwrap(),
         })
     }
 
@@ -322,6 +358,9 @@ impl GuestResource {
         Ok(Self {
             handle: self.handle.try_clone()?,
             planes: self.planes.clone(),
+            width: self.width,
+            height: self.height,
+            format: self.format,
         })
     }
 }