Fix simulated camera stalling

[aliddell]

If you've used the simulated camera for testing, especially with more resource intensive patterns like random or radial sine, you may have noticed that unless you set your exposure time to be very large, the camera can end up stalling and the runtime hangs, sometimes clearing up on its own but sometimes requiring you to kill the process.

The reason for this is because the rendering thread locks a mutex for most of its lifetime, releasing it briefly at the bottom of a while loop and reacquiring it at the top. A different thread, the source thread, tries to grab on to this mutex and fails to get it, but when it retries, the rendering thread has already grabbed it again. Eventually the source thread may succeed in locking the mutex, but several (often several hundred) iterations of the render thread's while loop have passed and we see that as dropped frames. When it happens over and over, performance is poor.

This PR modifies the work in the rendering thread to do the following:

• Use a separate thread-scoped buffer for rendering and memcpy the data into the camera thread's buffer.
• Wait on the software trigger before rendering, rather than after.
• Only lock the mutex when absolutely necessary, freeing it to be locked by the source thread (and subsequently unlocked when it waits on its condition variable).
• Keep track of time to render and apply it toward the overall exposure time. Sleep only for the time remaining.

[nclack]

I like the basic approach - minimizing the work inside the lock.

The approach looks like (a) set up a buffer for the rendering thread to use explicitly and (b) use the buffer in struct SimulatedCamera as a communication channel between the rendering thread and the client thread (whatever is calling get_frame).

Allocating a buffer inside the rendering thread has some problems - you need to handle failures in a way that wasn't present before. That's certainly possible, it just needs some thought.

There's an alternative. You don't need to have the rendering buffer owned by the thread. We only have one rendering thread per SimulatedCamera instance. You could allocate an extra buffer alongside the simulated context; an extra self->im.data with the same lifetimes etc. It looks a little like double buffering.

The upshot is that the memory allocation would happen simcam_set which already has a defined way to fail.

[nclack]

I made a couple suggestions. Pretty close!

I'm realizing there's another issue here. We'd like the device api to be thread-safe, and this one's not. That should be addressed outside of this pr. I will probably make an issue.

[nclack]

some more checked frees, and a suggestion

[nclack]

Probably worth factoring these code blocks into a function. Something like the suggestion below. One caveat is that the failure handling assumes you're inside the lock, so doing this requires modifying the top part of simcam_set as well.

Suggested change

	free(self->im.frame_data);
	self->im.frame_data = malloc(nbytes);
	if (!self->im.frame_data) {
	LOGE("Allocation of %llu bytes failed.", nbytes);
	lock_release(&self->im.lock);
	goto Error;
	}
	free(self->im.render_data);
	self->im.render_data = malloc(nbytes);
	if (!self->im.render_data) {
	LOGE("Allocation of %llu bytes failed.", nbytes);
	lock_release(&self->im.lock);
	goto Error;
	}
	lock_release(&self->im.lock);
	return Device_Ok;
	Error:
	return Device_Err;
	CHECK(checked_realloc(self->im.frame_data,nbytes);
	CHECK(checked_realloc(self->im.render_data,nbytes);
	int status=Device_Ok;
	Finalize:
	lock_release(&self->im.lock);
	return status;
	Error:
	status=Device_Err;
	goto Finalize;

where

void* checked_realloc(void* in,size_t nbytes) {
    // if `in` is NULL, realloc is identical to malloc.
    void* out = realloc(in,nbytes);
    EXPECT(out,"Allocation of %llu bytes failed",nbytes);
Error:
    return out;
}

[aliddell]

Alternative:

static void*
checked_realloc(void* in, size_t nbytes)
{
    void* out = realloc(in, nbytes);
    EXPECT(out, "Allocation of %llu bytes failed.", nbytes);

Finalize:
    return out;

Error:
    free(in);
    goto Finalize;
}

[nclack]

Probably worth searching around for other instances of these.

Suggested change

	if (camera->im.frame_data)
	free(camera->im.frame_data);
	if (camera->im.render_data)
	free(camera->im.render_data);
	free(camera->im.frame_data);
	free(camera->im.render_data);

[nclack]

lgtm!