From 49c8cd6160d3e4dfae93761c9c54ff13345f733e Mon Sep 17 00:00:00 2001
From: Alexandre Courbot <acourbot@chromium.org>
Date: Tue, 19 Jul 2022 11:59:18 +0900
Subject: [PATCH] media: ffmpeg: reduce dependency on base

We want the ffmpeg crate to be independent of crosvm - remove a
dependency to base by adding a dedicated trait for types that can be
used as sources to a ffmpeg buffer.

BUG=b:169295147
TEST=v4l2r's simple_decoder can decode a H.264 stream.

Change-Id: I2ae6c2678a6a672746e6fc11a70dee692986db69
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/3782032
Commit-Queue: Alexandre Courbot <acourbot@chromium.org>
Reviewed-by: Takaya Saeki <takayas@chromium.org>
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Tested-by: Alexandre Courbot <acourbot@chromium.org>
---
 .../virtio/video/decoder/backend/ffmpeg.rs    | 25 +++++++++++++++---
 media/ffmpeg/src/avcodec.rs                   | 26 ++++++++++++++-----
 2 files changed, 41 insertions(+), 10 deletions(-)

diff --git a/devices/src/virtio/video/decoder/backend/ffmpeg.rs b/devices/src/virtio/video/decoder/backend/ffmpeg.rs
index 0855943c4b..a1d102a83d 100644
--- a/devices/src/virtio/video/decoder/backend/ffmpeg.rs
+++ b/devices/src/virtio/video/decoder/backend/ffmpeg.rs
@@ -18,7 +18,7 @@
 use std::collections::{BTreeMap, VecDeque};
 
 use anyhow::{anyhow, Context};
-use base::{error, info, warn, MmapError};
+use base::{error, info, warn, MappedRegion, MemoryMappingArena, MmapError};
 use thiserror::Error as ThisError;
 
 use crate::virtio::video::{
@@ -32,6 +32,23 @@ use crate::virtio::video::{
 
 use ::ffmpeg::{avcodec::*, swscale::*, *};
 
+/// Structure maintaining a mapping for an encoded input buffer that can be used as a libavcodec
+/// buffer source.
+struct InputBuffer {
+    /// Memory mapping to the encoded input data.
+    mapping: MemoryMappingArena,
+}
+
+impl AvBufferSource for InputBuffer {
+    fn as_ptr(&self) -> *const u8 {
+        self.mapping.as_ptr()
+    }
+
+    fn len(&self) -> usize {
+        self.mapping.size()
+    }
+}
+
 /// All the parameters needed to queue an input packet passed to the codec.
 struct InputPacket {
     bitstream_id: i32,
@@ -189,10 +206,12 @@ impl FfmpegDecoderSession {
             bytes_used,
         } = input_packet;
 
-        let mapping = resource.get_mapping(*offset as usize, *bytes_used as usize)?;
+        let mut input_buffer = InputBuffer {
+            mapping: resource.get_mapping(*offset as usize, *bytes_used as usize)?,
+        };
 
         // Prepare our AVPacket and ask the codec to process it.
-        let avpacket = AvPacket::new(*bitstream_id as i64, &mapping);
+        let avpacket = AvPacket::new(*bitstream_id as i64, &mut input_buffer);
         match self.context.try_send_packet(&avpacket) {
             Ok(true) => Ok(true),
             // The codec cannot take more input at the moment, we'll try again after we receive some
diff --git a/media/ffmpeg/src/avcodec.rs b/media/ffmpeg/src/avcodec.rs
index cae8f6f437..3fd1245950 100644
--- a/media/ffmpeg/src/avcodec.rs
+++ b/media/ffmpeg/src/avcodec.rs
@@ -8,7 +8,6 @@
 
 use std::{ffi::CStr, fmt::Display, marker::PhantomData, ops::Deref};
 
-use base::MappedRegion;
 use libc::{c_char, c_int};
 use thiserror::Error as ThisError;
 
@@ -247,6 +246,19 @@ impl AvCodecContext {
     }
 }
 
+/// Trait for types that can be used as data provider for a `AVBuffer`.
+///
+/// `AVBuffer` is an owned buffer type, so all the type needs to do is being able to provide a
+/// stable pointer to its own data as well as its length. Implementors need to be sendable across
+/// threads because avcodec is allowed to use threads in its codec implementations.
+pub trait AvBufferSource: Send {
+    fn as_ptr(&self) -> *const u8;
+    fn as_mut_ptr(&mut self) -> *mut u8 {
+        self.as_ptr() as *mut u8
+    }
+    fn len(&self) -> usize;
+}
+
 /// An encoded input packet that can be submitted to `AvCodecContext::try_send_packet`.
 pub struct AvPacket<'a> {
     packet: ffi::AVPacket,
@@ -254,18 +266,18 @@ pub struct AvPacket<'a> {
 }
 
 impl<'a> AvPacket<'a> {
-    /// Create a new AvPacket.
+    /// Create a new AvPacket that borrows the `input_data`.
     ///
-    /// `input_data` is the encoded data we want to send to the codec for decoding. The data is not
-    /// copied by the AvPacket itself, however a copy may happen when the packet is submitted.
-    pub fn new<T: MappedRegion>(pts: i64, input_data: &'a T) -> Self {
+    /// The returned `AvPacket` will hold a reference to `input_data`, meaning that libavcodec might
+    /// perform a copy from/to it.
+    pub fn new<T: AvBufferSource>(pts: i64, input_data: &'a mut T) -> Self {
         Self {
             packet: ffi::AVPacket {
                 buf: std::ptr::null_mut(),
                 pts,
                 dts: AV_NOPTS_VALUE as i64,
-                data: input_data.as_ptr(),
-                size: input_data.size() as c_int,
+                data: input_data.as_mut_ptr(),
+                size: input_data.len() as c_int,
                 side_data: std::ptr::null_mut(),
                 pos: -1,
                 // Safe because all the other elements of this struct can be zeroed.