JuliaIO / VideoIO.jl

Reading and writing of video files in Julia via ffmpeg
https://juliaio.github.io/VideoIO.jl/stable
Other
128 stars 53 forks source link

Segmentation fault 11 after quit() following VideoIO opencamera()/close #44

Closed maxruby closed 10 years ago

maxruby commented 10 years ago

I observed consistently the following crash error after using quit() - only following use of f = VideoIO.opencamera("Built-in iSight", VideoIO.DEFAULT_CAMERA_FORMAT) Before using quit() to exit, the camera opened properly which I then close with close(f) System is OSX 10.9, Julia v0.3.

julia> quit()

signal (11): Segmentation fault: 11
av_opt_free at /usr/local/Cellar/ffmpeg/2.3.3/lib/libavutil.52.dylib (unknown line)
avcodec_close at /usr/local/Cellar/ffmpeg/2.3.3/lib/libavcodec.55.dylib (unknown line)
_close at /Users/maximilianosuster/.julia/v0.3/VideoIO/src/avio.jl:517
jlcall__close;20234 at  (unknown line)
jl_apply at /Users/maximilianosuster/julia/src/./julia.h:980
close at /Users/maximilianosuster/.julia/v0.3/VideoIO/src/avio.jl:524
jlcall_close;20232 at  (unknown line)
jl_apply at /Users/maximilianosuster/julia/src/./julia.h:980
jl_apply at /Users/maximilianosuster/julia/src/./julia.h:1174
run_finalizers at /Users/maximilianosuster/julia/src/gc.c:318
uv_atexit_hook at /Users/maximilianosuster/julia/src/init.c:447
jl_exit at /Users/maximilianosuster/julia/src/jl_uv.c:687
quit at client.jl:38
jlcall_quit;20235 at  (unknown line)
jl_apply at /Users/maximilianosuster/julia/src/gf.c:1416
jl_apply at /Users/maximilianosuster/julia/src/interpreter.c:59
eval at /Users/maximilianosuster/julia/src/interpreter.c:207
jl_toplevel_eval_flex at /Users/maximilianosuster/julia/src/toplevel.c:496
jl_f_top_eval at /Users/maximilianosuster/julia/src/builtins.c:396
eval_user_input at REPL.jl:54
jlcall_eval_user_input;19804 at  (unknown line)
jl_apply at /Users/maximilianosuster/julia/src/./julia.h:980
anonymous at task.jl:96
jl_apply at /Users/maximilianosuster/julia/src/task.c:427
julia_trampoline at /Users/maximilianosuster/julia/src/init.c:1007
Segmentation fault: 11
maxruby commented 10 years ago

Commenting out 523-525 in VideoIO/avio.jl removes the segmentation fault:

  for i in avin.listening
     _close(avin.stream_contexts[i+1])
  end

The following is a simple fix that avoids the segmentation fault:

 if length(avin.listening) > 1
       for i in avin.listening
        _close(avin.stream_contexts[i+1])
       end
  end
kmsquire commented 10 years ago

I wonder if the wrong streams are being closed. Maybe it's an off-by-1 bug?

maxruby commented 10 years ago

Changing the index in avin.stream_contexts[i+1] did not fix it. Also, I could not think of which other stream_contexts was opened except for the one in avin::AVInput.

In the REPL, avin.listening is IntSet[0]

 julia> f = VideoIO.opencamera("Built-in iSight", VideoIO.DEFAULT_CAMERA_FORMAT)
 julia> f.avin.listening
 IntSet([0])
 julia> length(f.avin.listening)
1
kmsquire commented 10 years ago

So here's what I think is happening: when you call close, it's closing the connection, but the list of connections being listened to isn't reset. Later, when Julia exits (or even the next time the garbage collector runs), close is called again (because it's a finalizer for the AVInput object, and it attempts to close the input again, which causes the segfault.

The solution would be to clear the list of inputs being listened to.

maxruby commented 10 years ago

Agreed. Actually, the following is a fix for segmentation fault issue #44:

avin.listening = IntSet()

for i in avin.listening
  _close(avin.stream_contexts[i+1])
end

Should I close this issue and submit this fix along with the pull request?

lucasb-eyer commented 10 years ago

Just fyi, if there's the string Fixes #44 in a commit of your PR, this issue will be automatically closed when the PR is accepted.

kmsquire commented 10 years ago
avin.listening = IntSet()

for i in avin.listening
  _close(avin.stream_contexts[i+1])
end

It needs to be the other way around: you want to close all of the streams, and then call empty!(avin.listening).

maxruby commented 10 years ago

I tried placing avin.listening = IntSet() after closing - that seemed to work too. Thank you for the clarification.

kmsquire commented 10 years ago

Either way would work. Using avin.listening = IntSet() is probably slightly more expensive, but this isn't in a critical path, so either way should work.

kmsquire commented 10 years ago

Please do create a seperate pull request/fix for just this issue.

maxruby commented 10 years ago

ok. will do!