renesas-rz / rz_linux-cip

Other
7 stars 20 forks source link

media: rzg2l-cru: Increase the number of V4L buffers compared to the number of HW slots #3

Closed WilliamBright-IMD closed 1 week ago

WilliamBright-IMD commented 2 weeks ago

g-streamer performs an V4L2_CID_MIN_BUFFERS_FOR_CAPTURE IOCTL to workout how many V4L buffers to allocate (https://github.com/GStreamer/gst-plugins-good/blob/20bbeb5e37666c53c254c7b08470ad8a00d97630/sys/v4l2/gstv4l2object.c#L820). The description of this parameter is the following:

This is a read-only control that can be read by the application and used as a hint to determine the number of CAPTURE buffers to pass to REQBUFS. The value is the minimum number of CAPTURE buffers that is necessary for hardware to work.

Before this patch, there's a 1:1 relationship between the used number of HW slots and the number of V4L buffers. Which means that there's no spare buffers to use for filling in slots after receiving a frame.

After this patch there's 3x more buffers than slots which means that there should always be many spare buffers that can be used for filling in slots after receiving a frame. The downside to this patch is that the number of slots is now hard-coded to be 4.

A way of reproducing this is issue is by using v4l-ctrl which allows you to control the number of buffers as a command line argument as g-streamer doesn't currently support setting the number of V4L buffers.

4 V4L buffer/4 slots test:

root@imdt-v2h-sbc:~# v4l2-ctl --stream-mmap=4  --set-fmt-video=width=1280,height=720 -d /dev/video0
<<<<<<<<<<<<<< 15.04 fps, dropped buffers: 3
<<<<<<<<<<<<< 15.93 fps, dropped buffers: 3
<<<<<<<<<<<<<<< 16.82 fps, dropped buffers: 4
<<<<<<<<<<<<<<< 17.42 fps, dropped buffers: 4

12 V4L buffers/4 slots test

root@imdt-v2h-sbc:~# v4l2-ctl --stream-mmap=12  --set-fmt-video=width=1280,height=720 -d /dev/video0
<<<<<<<<<<<<<<<<<<<<< 19.50 fps
<<<<<<<<<<<<<<<<<<<< 19.50 fps
<<<<<<<<<<<<<<<<<<< 19.50 fps
<<<<<<<<<<<<<<<<<<<< 19.50 fps
<<<<<<<
hienhuynh2809 commented 1 week ago

Hi WilliamBright-IMD, You just need to set the number of buffers equal or higher than the minimum required buffers when capturing. We set the number of buffers based on the number of Hardware buffers (memory bank). I try your command and there is no drop fps issue. They are still 30fps. Which camera are you using?

root@rzv2h-evk-ver1:~# v4l2-ctl --stream-mmap=4  --set-fmt-video=width=1280,height=960 -d /dev/video0
<<<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps, dropped buffers: 6
<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps, dropped buffers: 6
<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps, dropped buffers: 6
<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps, dropped buffers: 6
...
root@rzv2h-evk-ver1:~# v4l2-ctl --stream-mmap=8  --set-fmt-video=width=1280,height=960 -d /dev/video0
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps
...
v4l2-ctl --stream-mmap=12  --set-fmt-video=width=1280,height=960 -d /dev/video0
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< 30.02 fps

To set the minimum number of captured buffers, you can try below command:

Example for 6 buffers.
root@rzv2h-evk-ver1:~# v4l2-ctl -d /dev/video0 -cmin_number_of_capture_buffers=6
...
Recheck the result:
root@rzv2h-evk-ver1:~# v4l2-ctl -d /dev/video0 -l
...
  min_number_of_capture_buffers 0x00980927 (int)    : min=2 max=8 step=1 default=4 value=6
...
WilliamBright-IMD commented 1 week ago

@hienhuynh2809 In the first command you sent it looks like V4L ctrl is saying there's dropped buffers.

I'm using an AP1302 ISP with an AR1335.

Gstreamer doesn't have a parameter to set the number of V4L buffers or hardware buffers to use and relies on using a V4L2_CID_MIN_BUFFERS_FOR_CAPTURE command. And this command should return a valid number of buffers to allocate that won't cause frame drops. Hence I have done this change instead of patching gstreamer as other programs may also use V4L2_CID_MIN_BUFFERS_FOR_CAPTURE to calculate how many buffers to allocate.

hienhuynh2809 commented 1 week ago

Hi WilliamBright-IMD,

In the first command you sent it looks like V4L ctrl is saying there's dropped buffers.

You can set the number of buffers = minimum + 1.

I'm using an AP1302 ISP with an AR1335.

Which maximum framerate does AR1335 support when capturing HD?

Gstreamer doesn't have a parameter to set the number of V4L buffers or hardware buffers to use and relies on using a V4L2_CID_MIN_BUFFERS_FOR_CAPTURE command. And this command should return a valid number of buffers to allocate that won't cause frame drops.

Yes, it uses V4L2_CID_MIN_BUFFERS_FOR_CAPTURE to get the minimum then set +1 or +2 for the amount buffers to avoid drop frames.

I have done this change instead of patching gstreamer as other programs may also use V4L2_CID_MIN_BUFFERS_FOR_CAPTURE to calculate how many buffers to allocate.

Sorry, this means you do not use V4L2_CID_MIN_BUFFERS_FOR_CAPTURE, right?

WilliamBright-IMD commented 1 week ago

Which maximum framerate does AR1335 support when capturing HD?

I don't know the maximum limit, I know that its capable of 720P 30 FPS and 1080P 30 FPS. I can achieve these frame rates as long as I have more than 8 V4L Buffers allocated.

Yes, it uses V4L2_CID_MIN_BUFFERS_FOR_CAPTURE to get the minimum then set +1 or +2 for the amount buffers to avoid drop frames.

Where does it perform the +1 or +2 in the gstreamer code? I can't find a location in the code that does the +1 or +2. I did perform a test in gstreamer where I compared the frame rate before and after this patch. Before the patch I measured 26.5 FPS when performing H264 encoding, after this patch I measure 28.5 FPS when performing H264 encoding. You can test this with this pipeline: gst-launch-1.0 -e v4l2src device=/dev/video0 io-mode=4 ! video/x-raw, width=1920, height=1080, framerate=30/1, format=YUY2 ! queue ! vspmfilter dmabuf-use=true ! video/x-raw, format=NV12 ! omxh264enc control-rate=2 target-bitrate=10485760 interval_intraframes=14 periodicty-idr=2 ! video/x-h264, profile=\(string\)high, level=\(string\)4 ! h264parse ! video/x-h264, stream-format=avc, alignment=au ! qtmux ! filesink location=MP4_file_1080_30.mp4

Sorry, this means you do not use V4L2_CID_MIN_BUFFERS_FOR_CAPTURE, right?

Personally we don't use this command in our programs that we write but we expect that some users of our hardware may use V4L2_CID_MIN_BUFFERS_FOR_CAPTURE within their own software.

hienhuynh2809 commented 1 week ago

Hi WilliamBright-IMD,

Where does it perform the +1 or +2 in the gstreamer code? I can't find a location in the code that does the +1 or +2.

https://github.com/GStreamer/gst-plugins-good/blob/20bbeb5e37666c53c254c7b08470ad8a00d97630/sys/v4l2/gstv4l2object.c#L4907 From L4907 to L4941.

You can test this with this pipeline: gst-launch-1.0 -e v4l2src device=/dev/video0 io-mode=4 ! video/x-raw, width=1920, height=1080, framerate=30/1, format=YUY2 ! queue ! vspmfilter dmabuf-use=true ! video/x-raw, format=NV12 ! omxh264enc control-rate=2 target-bitrate=10485760 interval_intraframes=14 periodicty-idr=2 ! video/x-h264, profile=(string)high, level=(string)4 ! h264parse ! video/x-h264, stream-format=avc, alignment=au ! qtmux ! filesink location=MP4_file_1080_30.mp4

I tested this pipeline with/without your commit and the results are identical in both cases.

diff --git a/drivers/media/platform/rzg2l-cru/rzg2l-core.c b/drivers/media/platform/rzg2l-cru/rzg2l-core.c
index 3dc47756b46c..4790d93e144d 100644
--- a/drivers/media/platform/rzg2l-cru/rzg2l-core.c
+++ b/drivers/media/platform/rzg2l-cru/rzg2l-core.c
@@ -485,7 +485,7 @@ static int rzg2l_cru_s_ctrl(struct v4l2_ctrl *ctrl)
        switch (ctrl->id) {
        case V4L2_CID_MIN_BUFFERS_FOR_CAPTURE:
                if ((cru->state == STOPPED) || (cru->state == STOPPING))
-                       cru->num_buf = ctrl->val;
+                       cru->num_buf =  HW_BUFFER_DEFAULT;
                else
                        ret = -EBUSY;

@@ -564,7 +564,8 @@ static int rzg2l_cru_probe(struct platform_device *pdev)
        v4l2_ctrl_handler_init(&cru->ctrl_handler, 2);
        ctrl = v4l2_ctrl_new_std(&cru->ctrl_handler, &rzg2l_cru_ctrl_ops,
                          V4L2_CID_MIN_BUFFERS_FOR_CAPTURE,
-                         2, HW_BUFFER_MAX, 1, HW_BUFFER_DEFAULT);
+//                       2, HW_BUFFER_MAX, 1, HW_BUFFER_DEFAULT);
+                         12, 20, 1, 12);

        ctrl->flags &= ~V4L2_CTRL_FLAG_READ_ONLY;
root@rzv2h-evk-ver1:~# v4l2-ctl -d /dev/video0 -Cmin_number_of_capture_buffers
min_number_of_capture_buffers: 12

They are 26fps. So, there is no changes/update from your commit. I calculate the fps from filesink by adding "name=v -rp v:sink" at the end of your command. The drop framerate may relate to omxh264enc.

Personally we don't use this command in our programs that we write but we expect that some users of our hardware may use V4L2_CID_MIN_BUFFERS_FOR_CAPTURE within their own software.

So, your current gstreamer does not support this feature, right?

WilliamBright-IMD commented 1 week ago

Thanks @hienhuynh2809 for pointing out the location in gstreamer where the +1 or +2 happens.

Since the PR doesn't benefit your gstreamer pipeline, I will close the PR.

When I looked at why we had 26 FPS instead of 30 FPS, I found that the scratch buffer was being used by the CRU because there weren't any free V4L buffers. Maybe this is worth you checking? Which camera sensor do you use out of interest?

So, your current gstreamer does not support this feature, right?

It does support this feature

hienhuynh2809 commented 1 week ago

Hi WilliamBright-IMD,

Which camera sensor do you use out of interest?

I use google coral OV5645 for the testing. It can support FULLHD@30. https://coral.ai/products/camera

When I looked at why we had 26 FPS instead of 30 FPS, I found that the scratch buffer was being used by the CRU because there weren't any free V4L buffers

OMX encoder processes the V4L2 longer than expected. It does not free and notify the CRU driver on time so that frame drops. If I do not use the omxh264enc (use only vspmfilter), the fps is still good (30fps).