don't remove underlying VideoStream file when doing save() (#3883)

* don't remove underlying VideoStream file when doing save()

* remove VideoStream underlying file when cleaning the object

* Update CHANGELOG.md

---------

Co-authored-by: Simon <sdanisch@protonmail.com>

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).
 - Fix label/legend for plotlist [#4079](https://github.com/MakieOrg/Makie.jl/pull/4079).
 - Fix wrong order for colors in RPRMakie [#4098](https://github.com/MakieOrg/Makie.jl/pull/4098).
 - Fixed incorrect distance calculation in `pick_closest` in WGLMakie [#4082](https://github.com/MakieOrg/Makie.jl/pull/4082).

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

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
 
 """
@@ -267,7 +271,6 @@ function save(path::String, io::VideoStream; video_options...)
     else
         cp(io.path, path; force=true)
     end
-    rm(io.path)
     return path
 end