Video : Add frame interval APIs

[ngphibang]

Video devices usually supports changing frame rates. This PR adds set/get/enum frame intervals APIs to support this.
It also

• adds an implementation of the new APIs in the video software generator
• adds code to the video capture sample as an example of using these APIs

[ngphibang]

@loicpoulain : Hi Loic, could you please help to review this one ?

[ngphibang]

@loicpoulain

[josuah]

Note: I am not a maintainer, not a collaborator, and not even a contributor yet!
Just free-form feedback... In hope not to bring more overhead.

[decsny]

@josuah thank you for helping review the video PRs, it needs more engagement in zephyr from people who know about it

[loicpoulain]

@ngphibang why it could not be added as a generic control (set/get_ctrl)?

[ngphibang]

@loicpoulain
Get/Set_Ctrl with VIDEO_CID_ are used for "simple" settings like contrast, brightness, 3A, etc. which concerns only the camera sensor, I think.

It's the same reason that we don't use set/get_ctrl for format I think. Framerate changing needs an accordance between elements in a camera pipeline (sensor, mipi csi receiver, etc.). It also depends on the capacity of related elements (that's why we need video_enum_frmival() as well).

It also depends on which format chosen at runtime, so somehow a bit more complex than set/get_format ...

[kartben]

some documentation related comments

[dleach02]

@ngphibang can you address @kartben change requests

[josuah]

@ngphibang why it could not be added as a generic control (set/get_ctrl)?

As I understand it: the struct video_frmival_fie is complex: "what frame rates are available for this resolution and this format".

One way to "squeeze" that multi-field query it through CIDs would be with several queries:

video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_WIDTH, 1280);
video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_HEIGHT, 720);
video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_FORMAT, video_fourcc(y, u, y, 2));
for (int i = 0;; i++) {
        video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_IDX, i);
        if (video_get_ctrl(dev, VIDEO_CID_FRMIVAL, &result) != 0) {
                break;
        }
        /* do something with `struct result` */
}

[ngphibang]

@josuah

As I understand it: the struct video_frmival_fie is complex: "what frame rates are available for this resolution and this format".

This is just one part but the main reason that set/get format and set/get frame rate need their own APIs is that these things involve other elements in the camera pipeline not only the sensor, so the desired format (or frame rate) needs to be propagated along the pipeline as being said in my previous comment.

It's the same reason why don't we just use set/get_ctrl for format and everything ?. A part from this, perhaps there are also other reasons that I am not aware of that the Linux community does not use set/get_ctrl for everything but has set/get_format and set/get_frmival APIs.

One way to "squeeze" that multi-field query it through CIDs would be with several queries:

video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_WIDTH, 1280);
video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_HEIGHT, 720);
video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_FORMAT, video_fourcc(y, u, y, 2));
for (int i = 0;; i++) {
        video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_IDX, i);
        if (video_get_ctrl(dev, VIDEO_CID_FRMIVAL, &result) != 0) {
                break;
        }
        /* do something with `struct result` */
}

This is said itself why we should not use set/get_ctrl() for frame rate and format :-). Here, to set a framerate for a given format and resolution, you used 5 separate set/get_ctrl calls already to the (1st) driver :). Then, that's not all, these values need to be propagated to other drivers along the pipeline (as set/get_format) as well ..

Then, how the drivers know which width / height / format / index queries belong to which frame rate setting ? ... i.e., what if someone calls video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_WIDTH, 1280); and video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_WIDTH, 1920); (2 times) then video_set_ctrl(dev, VIDEO_CID_FRMIVAL_QUERY_HEIGHT, 720); (one time), for example ?

Perhaps you will be able to find another "workaround" to that ... but why we do that ? Why don't just make thing natural and intuitive with an API ?

It's like why we need a multiplication operator to do e.g. 2 * 3 while we could always do it with addition operators only : 2 + 2 + 2 :-)

[ngphibang]

Rebased due to #73001 is merged which moved the video samples from /subsys to /drivers.

@loicpoulain : Could you please help to review this as it has been here for some months ?

For your question, as explained above, get/set_format as well as get/set_frmival cannot be done using get/set_ctrl for sure.

Semantically, get/set_ctrl is used for settings that relates only to the camera sensor, not the whole pipeline while format and framerate concern the whole pipeline. When you change the format or the frame rate, not only the sensor has to be changed, but also the receiver (e.g. mipi csi2rx) has to be changed to afford it, i.e. format / framerate changes imply to changes in pixel rate, link frequency, etc.

Technically, it needs to be implemented as a pipeline API. For an example of a real usecase, this PR and this PR implement the frame rate changing for the csi -> mipi csi2rx -> ov5640 pipeline.

[dleach02]

@loicpoulain need your review on this PR. It is needed before a series of others that are pending.

[josuah]

Letting the conversation above get resolved. No issue on my end with this way of solving format negotiation. I better avoid open-ended discussions in the middle of reviews to not delay the "go".

[ngphibang]

@loicpoulain : Could you please revisit this PR to move it forward ?
Thanks !