driver: video: esp32: add video_flush() callback

[epc-ake]

This PR adds the video_flush() callback to the drivers API.

• If cancel is true, streaming is stopped and all buffers in the input FIFO are moved to the output FIFO.
• If cancel is false, video_flush() blocks until all enqueued input buffers are filled with incoming data

The application can then call video_dequeue() until no buffers remain enqueued.

[josuah]

Just some typo, for the rest, LGTM.
Thanks for the edits!

[ngphibang]

Sorry to comment a bit late when this is just merged but when video_esp32_flush() is called (by video_stream_stop()), the video_esp32_set_stream() is already called. This is the same error as in video_emul driver which I fixed here.

static inline int video_stream_stop(const struct device *dev)
{
	const struct video_driver_api *api = (const struct video_driver_api *)dev->api;
	int ret;

	if (api->set_stream == NULL) {
		return -ENOSYS;
	}

	ret = api->set_stream(dev, false);
	video_flush(dev, VIDEO_EP_ALL, true);

	return ret;
}

[josuah]

If the driver's flush() does not perform the needed "stop" actions, then the user should always call video_stream_stop() before video_flush()... But as video_stream_stop() already calls video_flush(), that suggests that video_flush() should be deprecated in the long run?

There remain the need to handle bool cancel... In that case, set_stream() would need to be a more generic operation maybe... Like set_stream(START), set_stream(STOP), set_stream(CANCEL) etc.

Maybe this comes as part of the video control framework PR, in which case I'll just wait patiently. :)

[ngphibang]

If the driver's flush() does not perform the needed "stop" actions, then the user should always call video_stream_stop() before video_flush()...

No, here we talk only the case cancel = true that means the flush callback is surely called by video_stream_stop() and in video_stream_stop(), set_stream(false) is already called before calling the flush, so no need to call it again.

[ngphibang]

For a broader scope of discussion about video_flush(), there's my long comment here.

Firstly, I think we need to agree on what to do to "flush" when stopping stream. It then depends on what we agree on what to do to restart. As widely adopted in Linux as a general rule (in my quoted comment), restart is like a new start, user needs to (re-)enqueue buffers. So, when flushing, both fifo_in and fifo_out need to be emptied / cleaned so that everything can be restarted again.

However, current implementation of flush callback in some Zephyr drivers like esp32, csi, sw_generator is just taking all the buffers in the fifo_in and put them to fifo_out. It needs to be rethinked.

Another difficulty is low level drivers (which Zephyr drivers are based on) do it differently, for example, the CSI low level driver puts all the buffers in fifo_out into the fifo_in (contracdictory with what Zephyr driver does) as it expected to be able to restart right away without re-enqueuing buffer (by user).

And Linux does not expose flushing API to driver but does it implicitly in the v4l2 framework ...

So it's a bit complicated. I will try to address this because it is also related to removing the video_endpoint_id in flushing API.

[ngphibang]

BTW, I don't see any usecase / need to force a flush when stream is on-going (cancel = false) ?

[josuah]

I will pursue on this PR if that makes sense...
#87070 (comment)

Thank you for your in-depth answer!