[Feature] Make gocv.VideoCapture thread-safe

[denismakogon]

So, the idea to make gocv.VideoCapture safe while accessing one from goroutines.
Here's the use case: as developer, i want to parse a video in frames (1 chunk equals to video's FPS) concurrently, the most efficient way to make this happen to use VideoCapture.Set and VideoCapture.Read.
This is a sample code to do a frame reading (not thread-safe):

func readFramesFromPosition(video *gocv.VideoCapture, startFrameIndex, framesToRead int64) ([]*gocv.Mat, error) {
	log := logrus.New()
	var mats []*gocv.Mat
	tmpMat := gocv.NewMat()

	video.Set(gocv.VideoCapturePosFrames, float64(startFrameIndex))
	defer video.Set(gocv.VideoCapturePosFrames, float64(0))
	log.Infof("reading frames for range: [%d:%d]", startFrameIndex, startFrameIndex+framesToRead)

	for i := int64(0); i < framesToRead; i++ {
		success := video.Read(&tmpMat)
		if !success {
			return nil, fmt.Errorf("unable to read frame on position: '%d' "+
				"from a video file", startFrameIndex+i+1)
		}
		mats = append(mats, &tmpMat)
	}
	return mats, nil
}

So, all i want is to start a bunch of these functions in parallel to speed up processing time because it make take hell of a time to process all frames from, let's say 1.4Gb, movie file.

My suggestion is:

• make an acess to Set and Read mutex-protected, similar to gocv.VideoWriter

breaf solution:

import (
	"sync"

	"gocv.io/x/gocv"
)

type SyncVideoCapture struct {
	mu *sync.RWMutex
	v gocv.VideoCapture
}

func (sv *SyncVideoCapture) Set(prop gocv.VideoCaptureProperties, param float64) {
	sv.mu.RLock()
	sv.v.Set(prop, param)
	sv.mu.RUnlock()
}

func (sv *SyncVideoCapture) Get(prop gocv.VideoCaptureProperties) float64 {
	sv.mu.Lock()
	defer sv.mu.Unlock()
	return sv.v.Get(prop)
}

func (sv *SyncVideoCapture) Read(m *gocv.Mat) bool {
	sv.mu.RLock()
	defer sv.mu.RUnlock()
	return sv.v.Read(m)
}

[denismakogon]

Here's fun thing. Doesn't seem like it's possible:

goroutine 44 [syscall]:
runtime.cgocall(0x79bbe0, 0xc420287d58, 0x8ade05)
	/usr/local/go/src/runtime/cgocall.go:128 +0x64 fp=0xc420287d10 sp=0xc420287cd8 pc=0x4219d4
function/vendor/gocv.io/x/gocv._Cfunc_VideoCapture_Set(0x7f7a00f1f580, 0x7f7a00000001, 0x4081300000000000)
	_cgo_gotypes.go:3545 +0x45 fp=0xc420287d58 sp=0xc420287d10 pc=0x640b95
function/vendor/gocv.io/x/gocv.(*VideoCapture).Set.func1(0x7f7a00f1f580, 0x1, 0x4081300000000000)
	/go/src/function/vendor/gocv.io/x/gocv/videoio.go:190 +0x6a fp=0xc420287d90 sp=0xc420287d58 pc=0x641f7a
function/vendor/gocv.io/x/gocv.(*VideoCapture).Set(0xc42009c1c0, 0x1, 0x4081300000000000)
	/go/src/function/vendor/gocv.io/x/gocv/videoio.go:190 +0x43 fp=0xc420287db8 sp=0xc420287d90 pc=0x641923

[deadprogram]

I think the "correct' pattern for OpenCV, and consequently GoCV, is probably to leave the capture in a single thread, but process the Mat structs read in from it in different threads aka Goroutines. According to OpenCV docs, a Mat is guaranteed to be thread-safe. I find no such assurance about VideoCapture.

@denismakogon what do you think?

[denismakogon]

That’s the problem I’ve faced recently. Even starting multiple video captures, one per each goroutine, it still fails to read frames concurrently.

[deadprogram]

Yeah, I'm not too sure this is possible, or needed, as long as one accepts using a different concurrency pattern.

[edwvee]

LockOSThread may help.

[deadprogram]

The "proper" concurrency pattern for this really seems to be a fan-out, using a single go routine to load in the Mats to be processed, and then multiple go routines to process each Mat. The underlying OpenCV video cap does not appear to be threadsafe the way you are trying to use in the example.

[marktheunissen]

Seems like it's a generally good idea to call LockOSThread when doing sensitive operations like capturing from a video source. I've been having issues with crashes that I assume are coming from gstreamer pipeline (dumping core shows call to VideoCapture.Read() hanging), and this change seems to have helped immediately, still more testing needs to be done, but it makes intuitive sense - if Go is pausing the capturing goroutine to reschedule it onto another thread, and that routine is capturing video through a C call to OpenCV which is in turn linking to a gstreamer pipeline, it's no wonder that the whole thing could crash. Anyway if it does continue to be stable I'll report back but preliminary results are good.

[iaburton]

I agree with @deadprogram, the fan-out technique with a channel and several goroutines is what I've used to accomplish this, I'd hate to see more mutexs peppered over Go code that doesn't need it (looking at you standard library).

About using LockOSThread, the problem isn't Go rescheduling a goroutine onto another thread while in a C call. It can't and has never done that. You can see here in this old issue (I've seen it other places too but this was one of the first when google'd) golang/go#8636 (comment), when a cgo call is made that thread is essentially owned by that C code (runtime can't just stop/preempt it like Go code), this is why in earlier versions of Go if you had lots of CGO calls you could accidentally create too many threads, though I believe newer versions fixed this (there was also talk about making LockOSThread mark the thread as "dirty" so other goroutines wouldn't run on it or spawn from it... don't remember if that was implemented).

The issue is once the cgo call is done and we're back in go-land, if the goroutine is put on another thread and the C call is made again and that C code was using thread local storage, then you can have a problem. That's basically the biggest hurdle when using Go with C graphics libraries right now, how often they use TLS.

Edit: Forgot to mention that in my use case, where I had one goroutine reading from the device and sending data over a channel for other goroutines to work on, I've never had a crash or other weird issue, and the one routine reading from the device was not using LockOSThread.

[deadprogram]

@iaburton that was very well stated, as far as the "correct" pattern to use, and the reasons why. Seems to me that this issue can be closed now, @denismakogon what do you think?

[marktheunissen]

Thanks for the info, agree about the diagnosis of thread-local storage being the problem.

Edit: Forgot to mention that in my use case, where I had one goroutine reading from the device and sending data over a channel for other goroutines to work on, I've never had a crash or other weird issue, and the one routine reading from the device was not using LockOSThread.

It might also be that Go runtime just happened to keep that goroutine on a single thread anyway. Only way to prove it doesn't affect it, would be to kill the thread, forcing the scheduler to start a new one for the goroutine.

[iaburton]

@marktheunissen That's true, such is the way of these non-deterministic things. My heaviest use time was in a couple of go/gocv/opencv versions ago, I suppose something could have changed recently in one of them too that might be causing something. Though I would still argue that it would need more investigation by those who have seen it, before adding mutexs to gocv.

I guess this is slightly OT but is there an easy way to kill a thread in Go? I didn't think there was.

[marktheunissen]

Yes it's possible through grabbing the id and just doing either a call directly through runtime or by calling some kind of intermediate function. I haven't done it before but it's possible.

[iaburton]

I had thought the problem was the goroutine could switch threads during/after the call to get the ID, making it useless, you could potentially kill the wrong one. I think this is why they didn't expose a simple method to get the ID in the standard library. I guess you could get around that with LockOSThread but I'd suspect the goroutine would die (or panic) with it if you tried to kill the thread it was locked to, defeating the purpose of the test you propose.

Would be interesting to find out though, not sure what'll happen.

[deadprogram]

As far as I understand it, I do not think that goroutines will switch what OS thread they are executing against.

The problem is that some software such as OpenCV (also SDL as a point of reference) on some platforms like macOS (also possibly might be on Windows, not completely sure) require that any GUI windows created are off of the main application thread. This is not a problem on Linux.

In any case, that is just to throw in my own 25 cents.

[iaburton]

That can definitely be the case if you're using the GUI functions of gocv/opencv, though I would hope the video capture code doesn't automatically force that restriction too (admittedly I don't know if it does). I actually had a Go program a couple of years ago that needed to have small notification icons in the desktop of Windows/Mac/Linux and you're correct, MacOS was the most restrictive, it wanted any GUI/drawing/windowing event to be from the main thread that the program started with, Windows actually doesn't care if it's the main thread, it just wants it all from the same thread, and Linux didn't care so much as long as you didn't cause a race/memory corruption.