drivers: video: video_stm32_dcmi: More control on memory buffers.

[KurtE]

Summary

When I run a some simple Arduino sketchs (ArduinoCore-zephyr), that loops reading in from a camera and then displays them on some Video device, like Giga Display shield, or ST7789, the display takes longer to display an image than it takes the camera to capture the next, I see a lot of messages
like:

[00:00:07.207,000] <dbg> video_stm32_dcmi: HAL_DCMI_FrameEventCallback: Failed to get buffer from fifo
[00:00:07.439,000] <dbg> video_stm32_dcmi: HAL_DCMI_FrameEventCallback: Failed to get buffer from fifo
[00:00:07.670,000] <dbg> video_stm32_dcmi: HAL_DCMI_FrameEventCallback: Failed to get buffer from fifo
[00:00:07.902,000] <dbg> video_stm32_dcmi: HAL_DCMI_FrameEventCallback: Failed to get buffer from fifo
...

And when I complete the drawing of an image, if I simply ask for an image, I get the oldest one, which could be really old...
Would prefer to get the most recent one...

Secondary: wondering if we continue to use the memcpy of the video buffer frame to one of the other buffers, if in some cases
we may desire to have these two buffers in different memory regions. For example, have the main camera buffer in the processors
main memory and the secondary ones in SDRAM..

Secondary question: occasionally I see some messages about memory corruption. Wondering are the kernel FIFO apis thread/interrupt safe?
That is, could some of these corruptions be caused by k_fifo_put or k_fifo_put code being interrupted. LIke in my example suppose when I completed using a frame to display, the enqueue call gets interrupted...

Describe the solution you'd like

Maybe add an option, that says when you don't have any more free buffers, reuse the oldest. Maybe something like: CONFIG_VIDEO_BUFFER_POOL_REUSE_OLDEST

Something like:

void HAL_DCMI_FrameEventCallback(DCMI_HandleTypeDef *hdcmi)
{
	struct video_stm32_dcmi_data *dev_data =
			CONTAINER_OF(hdcmi, struct video_stm32_dcmi_data, hdcmi);
	struct video_buffer *vbuf;

	HAL_DCMI_Suspend(hdcmi);

	vbuf = k_fifo_get(&dev_data->fifo_in, K_NO_WAIT);

#ifdef CONFIG_VIDEO_BUFFER_POOL_REUSE_OLDEST
	if (vbuf == NULL) {
		vbuf = k_fifo_get(&dev_data->fifo_out, K_NO_WAIT);
	}
#endif

	if (vbuf == NULL) {
		LOG_DBG("Failed to get buffer from fifo");
		goto resume;
	}

	vbuf->timestamp = k_uptime_get_32();
	memcpy(vbuf->buffer, dev_data->vbuf->buffer, vbuf->bytesused);

	k_fifo_put(&dev_data->fifo_out, vbuf);

resume:
	HAL_DCMI_Resume(hdcmi);
}

Again wondering about FIFO being safe...

Alternatives

For now might have the user code, for example in my example sketch, could for example, keep asking for buffers until it says it does not have any more and then return all but the last one which I display.

And/Or while the code is waiting for the display to finish, it keep timings and at certain intervals have it toss any available images.

Additional Context

Side note: I was trying to test the latest stuff out on the Nicla Vision, as this is the only board I have that has a zephyr example
for Video. But not sure if it is currently working?
#90326

That is if I run simple sample, like hello world on it, the kernel monitor window on the Serial pins shows:

*** Booting Zephyr OS build v4.1.0-7111-g4ec435165753 ***
Hello World! arduino_nicla_vision/stm32h747xx/m7

And on this example I see nothing, just the green LED is on...

[josuah]

CONFIG_VIDEO_BUFFER_POOL_REUSE_OLDEST describes the problem well it seems.

There is an attempt to switch to RTIO for the buffering mechanism to reduce the code needed for buffering.

• drivers: video: switch from custom buffer queues to RTIO #89858

Another parallel topic is to make video drivers manage their own memory so that applications only have to video_get_vbuf(dev, &vbuf) buffers and video_free_vbuf(vbuf) or something similar.

• [no issue for it yet]

This is an occasion to solve these problems in the new approach.

[josuah]

It seems like the idea is to continuously maintain a buffer ready and only put it in the completion queue when the user tells "I need a buffer now", with the latest data available on it. This works, but it seems like moving the application logic into the driver. Another application would want to use something different.

Another possibility could be to manage it at the outside of the driver:

• In a "camera" thread, continuously enqueue and dequeue buffers with video_dcmi_dev: as soon as a buffer is dequeued, put it in an application_fifo, and immediately remove it from application_fifo then re-enqueue it again back into DCMI. We now have application_fifo that constantly contains a recent struct video_buffer.

• On a "display" thread with stronger priority, call vbuf = k_fifo_get(application_fifo, K_FOREVER); to catch the latest buffer from the FIFO and display it. If there is no buffer available, it would wait until one becomes available (K_FOREVER).

Maybe there are more efficient ways but this shows the idea. Is that working according to what you hdd in mind?

[ngphibang]

Yes, I described this on another thread. Currently, in the video capture sample, operations are blocking, i.e. we wait for the camera outputs a buffer and queued it to the display. To make it work in parallel and hence satisfy the requirement of this issue, application can use its own buffer queue (LIFO), camera and display produces and consume buffers in two separate threads.

A more sophisticated way is to use framework like gstreamer and insert a queue element between the camera and the display elements.

[KurtE]

Thanks everyone, for feedback.
I have been playing around with what I mentioned in the first post of the discussion. I have it up at
https://github.com/KurtE/zephyr/tree/video_stm32_dcmi_reuse_buffers
I was waiting to see what people thought before or if to create a PR.

I did the the changes as simple as possible.

With the current setup with N buffers lets say 5 and there are 10 camera frames captured before the application asks for them.
It feels like there are two options: return the first 5 frames read (current code) or return the last 5, what the option would do.
In my case I an not look for LIFO as I only want the last one... I mainly use the camera code for the fun of it, so others may
have more sophisticated needs.

What I typically want, is one of two things, which I am guessing are pretty common:

a) Snapshot, that is like I push the take a picture now button, and the camera starts and return me the picture... One
buffer may do.

b) Semi-continuous: In this one the camera owns 2 buffers. The one it is writing to, and a second one, typically with
the last full frame read in. And the app owns a third buffer. When the app wants the last full image, it swaps it's buffer
with the last full frame buffer of the camera...

That is logically what I am doing here... Note: I am hoping that at some point, that we can get away from doing a
memcpy of one full buffer to another. For example for the images I am currently displaying 320x480 the
interrupt handler is copying: 307,200 bytes of image from one SDRAM buffer to another.
But I have not played enough with some of this to know how hard it would be to update the Camera DMA stream,
to output to a new memory location, during the frame completion interrupt...

As I mentioned in the first post, Alternative for me is to have my thread (or in the Arduino case, the main loop,
I can check for any available frames, and toss them, up until I think I am almost done with the previous image
I am outputting to the display.

I will show more about the simple sketch in the how to restart the camera issue...

[avolmat-st]

Hello,

sorry for jumping a bit late in this discussion.

I am puzzled about the fact of reusing an old (not get taken by the application) buffer. I mean, when we say that a buffer is already filled with data and made available to other level, we usually do not expected it to be modified again.
Sorry I forgot, could you remind me why you'd need a large number of buffers out of the DCMI ? As much buffer you'd have handled by the DCMI, as much latency you'll have. So on the opposite, since anyway it seems that the time to capture seems negligeable compared to the display time, is a single DCMI buffer (available to application) suffisant ? (aka 2 buffers as a whole).

A small optimization that could be done within the DCMI is to avoid calling the DCMI_Resume if there is no buffer available to receive the data. The DCMI_Resume would only be called as part of the enqueue of a new buffer if it is noticed that the driver in a state of pending for buffer (similar mechanism done in DCMIPP driver).

[KurtE]

Thanks @avolmat-st

In my case, or more specifically how the camera stuff was defined as part of the GIGA and Portenta H8, which I am playing with
right now... There are 3 buffers defined:

CONFIG_VIDEO_BUFFER_POOL_NUM_MAX=3
CONFIG_VIDEO_BUFFER_POOL_SZ_MAX=614400
CONFIG_VIDEO_BUFFER_POOL_ALIGN=32

At initialization time, the code currently grabs the first buffer and assigns it to the Camera to use to receive an image: The other two buffers are in the in FIFO queue.

The first image comes in, and the callback, then grabs a FB from the buffer pool and it does a memcpy to it and put's it into
the OUT fifo and continues. In this example case, the main thread then extracts the frame from the fifo and starts and starts
an output to the display which lets say takes half second to display.

I don't remember off hand how long it takes for the GC2145 to read a frame, but lets say we are receiving 10 frames per second, do during that half second to display a frame, the camera reads in 5 frames. The is only one frame left in the IN queue, which will
currently grab the first image, the subsequent 4 frames it will see that there are no buffers left in the IN FIFO and it punts
printing that message. The camera continue to do DMA operations as it holds onto that one buffer...

So when the display finishes it's output, The frame it will receive is about as old as when the previous frame started it's output
or about half second...

The other option I have without changing this code is to guestimate about how long it will be until the frame will complete and
then about that time - time to read a frame, it retrieves the current frame in the output queue and releases it back to the
input queue, which is doable.

And actually if one can not continue the camera until that point, it would probably work closer being able to use
the camera to take snapshots...

[avolmat-st]

Just an idea, doesn't "solve" the root cause in a nice way, however, if values "match" well, one way to reduce this delay between the moment the frame is taken and the moment you get from the video device would also be to reduce the framerate. Since anyway you know that you won't need a higher framerate than what the display can achieve, you can either change the framerate at sensor level, or have the help of the DCMI for that. It can reduce the framerate. In recent version of Zephyr this is now implemented directly via the frmival API. (it used to be available via a DT "capture-rate" property.

Sorry that doesn't solve the logic issue, but I thought it might help you as well.