fix(video): try to optimize memory leaks

crates/iced-video/Cargo.toml

@@ -16,3 +16,8 @@ tracing = "0.1.43"
 [dev-dependencies]
 iced.workspace = true
 tracing-subscriber = { version = "0.3.22", features = ["env-filter"] }
+
+[profile.dev]
+debug = true
+[profile.release]
+debug = true

crates/iced-video/src/primitive.rs

@@ -9,7 +9,7 @@ pub struct VideoFrame {
     pub id: id::Id,
     pub size: wgpu::Extent3d,
     pub ready: Arc<AtomicBool>,
-    pub frame: Arc<Mutex<Vec<u8>>>,
+    pub frame: Arc<Mutex<gst::app::Sample>>,
 }
 
 impl iced_wgpu::Primitive for VideoFrame {
@@ -34,6 +34,12 @@ impl iced_wgpu::Primitive for VideoFrame {
                 usage: wgpu::TextureUsages::TEXTURE_BINDING | wgpu::TextureUsages::COPY_DST,
                 view_formats: &[],
             });
+            let buffer = device.create_buffer(&wgpu::BufferDescriptor {
+                label: Some("iced-video-buffer"),
+                size: (self.size.width * self.size.height * 4) as u64,
+                usage: wgpu::BufferUsages::COPY_SRC | wgpu::BufferUsages::COPY_DST,
+                mapped_at_creation: false,
+            });
             let bind_group = device.create_bind_group(&wgpu::BindGroupDescriptor {
                 label: Some("iced-video-texture-bind-group"),
                 layout: &pipeline.bind_group_layout,
@@ -53,6 +59,7 @@ impl iced_wgpu::Primitive for VideoFrame {
             VideoTextures {
                 id: self.id.clone(),
                 texture,
+                buffer,
                 bind_group,
                 ready: Arc::clone(&self.ready),
             }
@@ -89,35 +96,21 @@ impl iced_wgpu::Primitive for VideoFrame {
             video.texture = new_texture;
             video.bind_group = new_bind_group;
         }
-        // BUG: This causes a panic because the texture size is not correct for some reason.
         if video.ready.load(std::sync::atomic::Ordering::SeqCst) {
+            let now = std::time::Instant::now();
             let frame = self.frame.lock().expect("BUG: Mutex poisoned");
-            if frame.len() != (4 * self.size.width * self.size.height) as usize {
-                tracing::warn!(
-                    "Frame size mismatch: expected {}, got {}",
-                    4 * self.size.width * self.size.height,
-                    frame.len()
-                );
-                return;
-            }
-            queue.write_texture(
-                wgpu::TexelCopyTextureInfo {
-                    texture: &video.texture,
-                    mip_level: 0,
-                    origin: wgpu::Origin3d::ZERO,
-                    aspect: wgpu::TextureAspect::All,
-                },
-                &frame,
-                wgpu::TexelCopyBufferLayout {
-                    offset: 0,
-                    bytes_per_row: Some(4 * video.texture.size().width),
-                    rows_per_image: Some(video.texture.size().height),
-                },
-                self.size,
-            );
+            let buffer = frame
+                .buffer()
+                .expect("BUG: Failed to get frame data from gst::Sample");
+
+            let data = buffer
+                .map_readable()
+                .expect("BUG: Failed to map gst::Buffer readable");
+            queue.write_buffer(&video.buffer, 0, &data);
             video
                 .ready
                 .store(false, std::sync::atomic::Ordering::SeqCst);
+            tracing::info!("{:?} Taken to write to surface texture", now.elapsed());
         }
     }
 
@@ -126,11 +119,29 @@ impl iced_wgpu::Primitive for VideoFrame {
         pipeline: &Self::Pipeline,
         encoder: &mut wgpu::CommandEncoder,
         target: &wgpu::TextureView,
-        clip_bounds: &iced_wgpu::core::Rectangle<u32>,
+        _clip_bounds: &iced_wgpu::core::Rectangle<u32>,
     ) {
         let Some(video) = pipeline.videos.get(&self.id) else {
             return;
         };
+
+        encoder.copy_buffer_to_texture(
+            wgpu::TexelCopyBufferInfo {
+                buffer: &video.buffer,
+                layout: wgpu::TexelCopyBufferLayout {
+                    offset: 0,
+                    bytes_per_row: Some(4 * self.size.width),
+                    rows_per_image: Some(self.size.height),
+                },
+            },
+            wgpu::TexelCopyTextureInfo {
+                texture: &video.texture,
+                mip_level: 0,
+                origin: wgpu::Origin3d::ZERO,
+                aspect: wgpu::TextureAspect::All,
+            },
+            self.size,
+        );
         let mut render_pass = encoder.begin_render_pass(&wgpu::RenderPassDescriptor {
             label: Some("iced-video-render-pass"),
             color_attachments: &[Some(wgpu::RenderPassColorAttachment {
@@ -164,6 +175,7 @@ impl iced_wgpu::Primitive for VideoFrame {
 pub struct VideoTextures {
     id: id::Id,
     texture: wgpu::Texture,
+    buffer: wgpu::Buffer,
     bind_group: wgpu::BindGroup,
     ready: Arc<AtomicBool>,
 }

crates/iced-video/src/source.rs

@@ -13,11 +13,11 @@ use std::sync::{Arc, Mutex, atomic::AtomicBool};
 #[derive(Debug, Clone)]
 pub struct VideoSource {
     pub(crate) playbin: Playbin3,
-    pub(crate) videoconvert: VideoConvert,
+    // pub(crate) videoconvert: VideoConvert,
     pub(crate) appsink: AppSink,
     pub(crate) bus: Bus,
     pub(crate) ready: Arc<AtomicBool>,
-    pub(crate) frame: Arc<Mutex<Vec<u8>>>,
+    pub(crate) frame: Arc<Mutex<gst::app::Sample>>,
 }
 
 impl VideoSource {
@@ -26,67 +26,51 @@ impl VideoSource {
     /// now.
     pub fn new(url: impl AsRef<str>) -> Result<Self> {
         Gst::new();
-        let videoconvert = VideoConvert::new("iced-video-convert")
-            // .change_context(Error)?
-            // .with_output_format(gst::plugins::videoconvertscale::VideoFormat::Rgba)
-            .change_context(Error)?;
+        // let videoconvert = VideoConvert::new("iced-video-convert")
+        //     // .change_context(Error)?
+        //     // .with_output_format(gst::plugins::videoconvertscale::VideoFormat::Rgba)
+        //     .change_context(Error)?;
         let appsink = AppSink::new("iced-video-sink")
             .change_context(Error)?
-            .with_caps(
-                Caps::builder(CapsType::Video)
-                    .field("format", "RGBA")
-                    .build(),
-            );
-        let video_sink = videoconvert.link(&appsink).change_context(Error)?;
+            .with_drop(true);
+        // .with_caps(
+        //     Caps::builder(CapsType::Video)
+        //         .field("format", "RGBA")
+        //         .build(),
+        // );
+        // let video_sink = videoconvert.link(&appsink).change_context(Error)?;
         let playbin = gst::plugins::playback::Playbin3::new("iced-video")
             .change_context(Error)?
             .with_uri(url.as_ref())
-            .with_video_sink(&video_sink);
+            .with_buffer_duration(core::time::Duration::from_secs(2))
+            .with_buffer_size(2000000)
+            .with_video_sink(&appsink);
         let bus = playbin.bus().change_context(Error)?;
         playbin.pause().change_context(Error)?;
         let ready = Arc::new(AtomicBool::new(false));
-        let frame = Arc::new(Mutex::new(Vec::new()));
+        let frame = Arc::new(Mutex::new(gst::app::Sample::new()));
 
         let appsink = appsink.on_new_frame({
             let ready = Arc::clone(&ready);
             let frame = Arc::clone(&frame);
             move |appsink| {
                 let Ok(sample) = appsink.pull_sample() else {
+                    tracing::error!("Failed to pull video sample from appsink despite being notified of new frame");
                     return Ok(());
                 };
-                let caps = sample.caps().ok_or(gst::gstreamer::FlowError::Error)?;
-                let structure_0 = caps.structure(0).ok_or(gst::gstreamer::FlowError::Error)?;
-                let width = structure_0
-                    .get::<i32>("width")
-                    .map_err(|_| gst::gstreamer::FlowError::Error)?;
-                let height = structure_0
-                    .get::<i32>("height")
-                    .map_err(|_| gst::gstreamer::FlowError::Error)?;
-
-                let buffer = sample.buffer().and_then(|b| b.map_readable().ok());
-                if let Some(buffer) = buffer {
-                    {
-                        let mut frame = frame.lock().expect("BUG: Mutex poisoned");
-                        debug_assert_eq!(buffer.size(), (width * height * 4) as usize);
-                        if frame.len() != buffer.size() {
-                            frame.resize(buffer.size(), 0);
-                        }
-                        frame.copy_from_slice(buffer.as_slice());
-                        ready.store(true, std::sync::atomic::Ordering::Relaxed);
-                    }
-                    // if written.is_err() {
-                    //     tracing::error!("Failed to write video frame to buffer");
-                    // } else {
-                    //     ready.store(true, std::sync::atomic::Ordering::Relaxed);
-                    // }
+                {
+                    let mut guard = frame.lock().expect("BUG: Mutex poisoned");
+                    core::mem::replace(&mut *guard, sample);
+                    ready.store(true, std::sync::atomic::Ordering::Relaxed);
                 }
+
                 Ok(())
             }
         });
 
         Ok(Self {
             playbin,
-            videoconvert,
+            // videoconvert,
             appsink,
             bus,
             ready,

flake.nix

@@ -196,11 +196,15 @@
                     lld
                     lldb
                     cargo-audit
+                    (crates.buildCrate "cargo-with" {doCheck = false;})
                   ]
                   ++ (lib.optionals pkgs.stdenv.isDarwin [
                     apple-sdk_26
                   ])
                   ++ (lib.optionals pkgs.stdenv.isLinux [
+                    valgrind
+                    hotspot
+                    samply
                     cargo-flamegraph
                     perf
                     mold

gst/src/plugins/app/appsink.rs

@@ -40,6 +40,11 @@ impl AppSink {
         self
     }
 
+    pub fn with_drop(self, drop: bool) -> Self {
+        self.inner.set_property("drop", drop);
+        self
+    }
+
     pub fn with_caps(self, caps: Caps) -> Self {
         self.inner.set_property("caps", caps.inner);
         self
@@ -111,12 +116,20 @@ impl From<gstreamer::Sample> for Sample {
 }
 
 #[repr(transparent)]
+#[derive(Debug, Clone)]
 pub struct Sample {
-    inner: gstreamer::Sample,
+    pub inner: gstreamer::Sample,
 }
 
 use gstreamer::BufferRef;
 impl Sample {
+    #[doc(alias = "empty")]
+    pub fn new() -> Self {
+        Self {
+            inner: gstreamer::Sample::builder().build(),
+        }
+    }
+
     pub fn buffer(&self) -> Option<&BufferRef> {
         self.inner.buffer()
     }
@@ -124,6 +137,14 @@ impl Sample {
     pub fn caps(&self) -> Option<&gstreamer::CapsRef> {
         self.inner.caps()
     }
+
+    pub fn info(&self) -> Option<&gstreamer::StructureRef> {
+        self.inner.info()
+    }
+
+    // pub fn set_buffer(&mut self) {
+    //     self.inner.set_buffer(None);
+    // }
 }
 
 #[test]

gst/src/plugins/playback/playbin3.rs

@@ -25,6 +25,24 @@ impl Playbin3 {
         self
     }
 
+    pub fn with_buffer_duration(self, duration: impl Into<Option<core::time::Duration>>) -> Self {
+        let duration = match duration.into() {
+            Some(dur) => dur.as_secs() as i64,
+            None => -1,
+        };
+        self.inner.set_property("buffer-duration", duration);
+        self
+    }
+
+    pub fn with_buffer_size(self, size: impl Into<Option<u32>>) -> Self {
+        let size = match size.into() {
+            Some(size) => size as i32,
+            None => -1,
+        };
+        self.inner.set_property("buffer-size", size);
+        self
+    }
+
     pub fn with_video_sink(self, video_sink: &impl ChildOf<Element>) -> Self {
         self.inner
             .set_property("video-sink", &video_sink.upcast_ref().inner);