From 1c962fc8991518ac514e590cb0fa0313c78878be Mon Sep 17 00:00:00 2001
From: Alexandre Courbot <acourbot@chromium.org>
Date: Wed, 2 Nov 2022 12:56:26 +0900
Subject: [PATCH] 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>
---
 .../cros-codecs/src/decoders/h264/picture.rs  | 70 +++++++++++++++----
 1 file changed, 57 insertions(+), 13 deletions(-)

diff --git a/media/cros-codecs/src/decoders/h264/picture.rs b/media/cros-codecs/src/decoders/h264/picture.rs
index 5f2ab0d13d..07cf0abaa3 100644
--- a/media/cros-codecs/src/decoders/h264/picture.rs
+++ b/media/cros-codecs/src/decoders/h264/picture.rs
@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 use std::cell::RefCell;
+use std::ops::Deref;
+use std::ops::DerefMut;
 use std::rc::Rc;
 use std::rc::Weak;
 
@@ -65,7 +67,7 @@ impl Default for IsIdr {
     }
 }
 
-pub struct Picture<BackendHandle> {
+pub struct PictureData<BackendHandle> {
     pub pic_order_cnt_type: u8,
     pub top_field_order_cnt: i32,
     pub bottom_field_order_cnt: i32,
@@ -110,6 +112,11 @@ pub struct Picture<BackendHandle> {
 
     is_second_field: bool,
     other_field: Option<Weak<RefCell<Picture<BackendHandle>>>>,
+}
+
+pub struct Picture<BackendHandle> {
+    /// Codec-specific data for this picture.
+    pub data: PictureData<BackendHandle>,
 
     /// The backend handle with any data the backend needs in order to back this
     /// picture.
@@ -121,9 +128,8 @@ pub struct Picture<BackendHandle> {
 
 impl<BackendHandle> Picture<BackendHandle> {
     pub fn new_non_existing(frame_num: i32, timestamp: u64) -> Self {
-        Self {
+        let data = PictureData {
             frame_num,
-            timestamp,
             nonexisting: true,
             nal_ref_idc: 1,
             field: Field::Frame,
@@ -153,7 +159,12 @@ impl<BackendHandle> Picture<BackendHandle> {
             is_second_field: Default::default(),
             other_field: Default::default(),
             ref_pic_marking: Default::default(),
+        };
+
+        Self {
+            data,
             backend_handle: Default::default(),
+            timestamp,
         }
     }
 
@@ -228,7 +239,7 @@ impl<BackendHandle> Picture<BackendHandle> {
             height: visible_rect.max.y - visible_rect.min.y,
         };
 
-        Self {
+        let pic_data = PictureData {
             pic_order_cnt_type: sps.pic_order_cnt_type(),
             pic_order_cnt_lsb: i32::from(pic_order_cnt_lsb),
             delta_pic_order_cnt_bottom,
@@ -243,6 +254,11 @@ impl<BackendHandle> Picture<BackendHandle> {
             ref_pic_marking: hdr.dec_ref_pic_marking().clone(),
             coded_resolution,
             display_resolution,
+            ..Default::default()
+        };
+
+        Self {
+            data: pic_data,
             timestamp,
             ..Default::default()
         }
@@ -303,13 +319,16 @@ impl<BackendHandle> Picture<BackendHandle> {
         }
 
         let mut other_field = Self {
-            top_field_order_cnt: pic.top_field_order_cnt,
-            bottom_field_order_cnt: pic.bottom_field_order_cnt,
-            frame_num: pic.frame_num,
-            reference: pic.reference,
-            nonexisting: pic.nonexisting,
-            pic_order_cnt,
-            field,
+            data: PictureData {
+                top_field_order_cnt: pic.top_field_order_cnt,
+                bottom_field_order_cnt: pic.bottom_field_order_cnt,
+                frame_num: pic.frame_num,
+                reference: pic.reference,
+                nonexisting: pic.nonexisting,
+                pic_order_cnt,
+                field,
+                ..Default::default()
+            },
             ..Default::default()
         };
 
@@ -378,11 +397,10 @@ impl<BackendHandle> Picture<BackendHandle> {
     }
 }
 
-impl<BackendHandle> Default for Picture<BackendHandle> {
+impl<BackendHandle> Default for PictureData<BackendHandle> {
     // See https://github.com/rust-lang/rust/issues/26925
     fn default() -> Self {
         Self {
-            backend_handle: None,
             pic_order_cnt_type: Default::default(),
             top_field_order_cnt: Default::default(),
             bottom_field_order_cnt: Default::default(),
@@ -413,6 +431,16 @@ impl<BackendHandle> Default for Picture<BackendHandle> {
             ref_pic_marking: Default::default(),
             is_second_field: Default::default(),
             other_field: Default::default(),
+        }
+    }
+}
+
+impl<BackendHandle> Default for Picture<BackendHandle> {
+    // See https://github.com/rust-lang/rust/issues/26925
+    fn default() -> Self {
+        Self {
+            data: Default::default(),
+            backend_handle: Default::default(),
             timestamp: Default::default(),
         }
     }
@@ -470,6 +498,22 @@ impl<BackendHandle> std::fmt::Debug for Picture<BackendHandle> {
     }
 }
 
+/// Give direct access to `data`'s members from a regular Picture as the H.264 code assumed these
+/// members were inlined.
+impl<BackendHandle> Deref for Picture<BackendHandle> {
+    type Target = PictureData<BackendHandle>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.data
+    }
+}
+
+impl<BackendHandle> DerefMut for Picture<BackendHandle> {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.data
+    }
+}
+
 impl<BackendHandle: MappableHandle> DynPicture for Picture<BackendHandle> {
     fn dyn_mappable_handle(&self) -> &dyn MappableHandle {
         self.backend_handle.as_ref().unwrap()