From f1bc65470b1e93b04392ab6d3cc095068a10723a Mon Sep 17 00:00:00 2001 From: Alexander Plavin Date: Thu, 23 May 2024 13:46:17 -0600 Subject: [PATCH 1/3] don't remove underlying VideoStream file when doing save() --- src/ffmpeg-util.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ffmpeg-util.jl b/src/ffmpeg-util.jl index ec280481b85..7744b8b6e44 100644 --- a/src/ffmpeg-util.jl +++ b/src/ffmpeg-util.jl @@ -267,7 +267,6 @@ function save(path::String, io::VideoStream; video_options...) else cp(io.path, path; force=true) end - rm(io.path) return path end From 6deae58e838b35b3e44b341233126da839616691 Mon Sep 17 00:00:00 2001 From: Alexander Plavin Date: Tue, 16 Jul 2024 16:22:48 -0400 Subject: [PATCH 2/3] remove VideoStream underlying file when cleaning the object --- CairoMakie/test/runtests.jl | 14 ++++++++++++-- src/ffmpeg-util.jl | 8 ++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/CairoMakie/test/runtests.jl b/CairoMakie/test/runtests.jl index 49158cdb546..ec52875c633 100644 --- a/CairoMakie/test/runtests.jl +++ b/CairoMakie/test/runtests.jl @@ -123,15 +123,25 @@ end N = 3 points = Observable(Point2f[]) f, ax, pl = scatter(points, axis=(type=Axis, aspect=DataAspect(), limits=(0.4, N + 0.6, 0.4, N + 0.6),), figure=(size=(600, 800),)) + vio = Makie.VideoStream(f; format="mp4", px_per_unit=2.0, backend=CairoMakie) + tmp_path = vio.path + @test vio.screen isa CairoMakie.Screen{CairoMakie.IMAGE} @test size(vio.screen) == size(f.scene) .* 2 @test vio.screen.device_scaling_factor == 2.0 Makie.recordframe!(vio) save("test.mp4", vio) - @test isfile("test.mp4") # Make sure no error etc - rm("test.mp4") + save("test_2.mkv", vio) + save("test_3.mp4", vio) + # make sure all files are correctly saved: + @test all(isfile, ["test.mp4", "test_2.mkv", "test_3.mp4"]) + @test filesize("test.mp4") == filesize("test_3.mp4") > 3000 + @test filesize("test.mp4") != filesize("test_2.mkv") > 3000 + rm.(["test.mp4", "test_2.mkv", "test_3.mp4"]) + finalize(vio); yield() + @test !isfile(tmp_path) end @testset "plotlist no ambiguity (#4038)" begin diff --git a/src/ffmpeg-util.jl b/src/ffmpeg-util.jl index 7744b8b6e44..322c09d4406 100644 --- a/src/ffmpeg-util.jl +++ b/src/ffmpeg-util.jl @@ -178,7 +178,7 @@ function to_ffmpeg_cmd(vso::VideoStreamOptions, xdim::Integer=0, ydim::Integer=0 end -struct VideoStream +mutable struct VideoStream io::Base.PipeEndpoint process::Base.Process screen::MakieScreen @@ -229,7 +229,11 @@ function VideoStream(fig::FigureLike; vso = VideoStreamOptions(format, framerate, compression, profile, pixel_format, loop, loglevel, "pipe:0", true) cmd = to_ffmpeg_cmd(vso, xdim, ydim) process = open(`$(FFMPEG_jll.ffmpeg()) $cmd $path`, "w") - return VideoStream(process.in, process, screen, buffer, abspath(path), vso) + result = VideoStream(process.in, process, screen, buffer, abspath(path), vso) + finalizer(result) do x + @async rm(x.path; force=true) + end + return result end """ From 176db76910deb1dd9a99a53d60a3ad5864cfdd40 Mon Sep 17 00:00:00 2001 From: Simon Date: Mon, 5 Aug 2024 20:26:54 +0200 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ffc746c084e..da03812aa9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased] +- don't remove underlying VideoStream file when doing save() [#3883](https://github.com/MakieOrg/Makie.jl/pull/3883). - Fixed incorrect distance calculation in `pick_closest` in WGLMakie [#4082](https://github.com/MakieOrg/Makie.jl/pull/4082). - Suppress keyboard shortcuts and context menu in JupyterLab output [#4068](https://github.com/MakieOrg/Makie.jl/pull/4068). - Introduce stroke_depth_shift + forward normal depth_shift for Poly [#4058](https://github.com/MakieOrg/Makie.jl/pull/4058).