-
Notifications
You must be signed in to change notification settings - Fork 8k
video: add more resolutions to GC2145 #91975
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
eed540f
to
145641c
Compare
video: add more resolutions to GC2145 As I mentioned in issue zephyrproject-rtos#91968, It would be great if the camera drivers would support a larger number of resolutions. For example those resolutions of the displays that they wish to display an image on. Potentially in both landscape and portrait mode. So far I have added one the works in landscape mode for ILI948x and ST7796 displays as well as a QVGA in portrait mode, which should be good for LI9341 and some ST7789 displays. If this looks like a valid approach, I will also add the GIGA Display Adapter size. Probably need to add the duplicate items to the FORMAT_CAP list for YUYV mode. Note: the other than add the items to the list, I just added code to set resolution which computes the c_ratio/r_ratio chooses the smaller of the two to use. We may also remove most if not all of the items in the switch statement and simply use the default clause code for them. Signed-off-by: Kurt Eckhardt <[email protected]>
145641c
to
9e291b5
Compare
|
c_ratio = UXGA_HSIZE / w; | ||
r_ratio = UXGA_VSIZE / h; | ||
if (c_ratio < r_ratio) { | ||
r_ratio = c_ratio; | ||
} else { | ||
c_ratio = r_ratio; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like some convenient "catch-all" rule to configure the exact resolution one needs but there is another way to do it:
-
The role of
video_set_format()
is to configure the "effective resolution", the size after binning is appplied (the row and column ratio like you configure now). -
The role of
video_set_selection()
is to apply a cropping to the "effective/binned resolution" into an "output resolution" to give full control over what the sensor supports.
This should be merged this week so if it is possible for you to wait (or implement it!), then this would allow you to control this sensor features fully.
Thank you for the proposal!
@josuah @dkalowsk @mjs513 and others... As I mentioned when I first submitted this, I believe it would be a good thing to add some more commonly used And the set_resolution procesing only allows these three sizes: Currently I am experimenting adding the functionality of video_get_selection/video_set_selection
I started with simple, just set the VIDEO_SEL_TGT_CROP in a test sketch to 480x320 and select 640x480 resolution and But if I try some of the updated video sample sketches, that use these messages, I see I need to handle some of the For example suppose I wish to configure the GC2145 for something like 800 x 480 (GIGA Display Shield) And assuming b) If I do get of VIDEO_SEL_TGT_NATIVE_SIZE and current resolution is set to 640x480, does it return c) if for example I have resolution: 640x480 and crop of 480x320, and set left and top to not be 0, I assume this d)Looking at some of the example code, And then looking at DCMIPP.c I believe that After I set a TGT_CROP an dthen e) Do I have additional control over the zoom level: That is suppose I wish for one of the resolution: 320x240 f) More of a side question: If the camera code supports the panning and zooming and so does the DCMIPP. Should DCMIPP (and other implementations) defer some of this to the cameras that support this? For example If I wish for for 480x320 where I have to choose 640x480, having the camera do it, would imply that it would clock out half as much data to the processor... Thanks in advance for insights here. I will continue to add in more of this support to the GC2145... |
The ideal would be, to focus on the hardware capabilities: the format selection is expected to never change the field of view. For sensors that support cropping (most do), it is further possible to leverage the Then it becomes clear what to implement (defined according to the hardware), and how to use he APIs:
The APIs are always called in this order: first the base resolution and then crop.
I have to check but this sounds like harware-dependent. Some video hardware will allow adding padding anywhere around the video feed.
=> Ideally, 800x600 (though could be 640x480 currently) Maybe 640x480 should be removed from the driver list of formats altogether! :D To get 640x480, one must select a native size of 800x600 then crop. This is not about Zephyr APIs but about what the hardware needs to do, no way around it... So the APIs need to reflect what the hardware does.
The rectangle is always absolute to the native format, and specifies the origin and the size: struct video_rect {
/** left offset of selection rectangle */
uint32_t left;
/** top offset of selection rectangle */
uint32_t top;
/** width of selection rectangle */
uint32_t width;
/** height of selection rectangle */
uint32_t height;
}; If you wish to pan, change the origin to a different value, this is not recursive so should work.
You may ignore "compose" (scale) in this case as not supported by DCMI, but this seems right...
This might make more sense when considering there is only one particular order users are expected to follow:
Every step reset the values of what is below it: "select the native size, remove margins, and scale it up/down" always in this order.
"Zoom" is a very high-level feature that comes from the application. Zooming is cropping into an image, but up-scaling it to compensate the smaller output resolution. In a DCMI-based hardware (i.e. no hardware scaler engine), how to perform zooming? The only 'scaling factors' we have are through software [wink] or through changing the sensor scaling factor. Then we can crop this output to get the desired output size. API-wise: select a different native resolution, then apply a different level of cropping. Suddenly we realize the importance of not changing the field of view (cropping) at the level of the sensor
It is clever for an application to do this, but drivers are probably not a good place for clever decisions... and what if different drivers take different decisions... Best is to move all these decisions into the application, or eventually intermediate layers where these kind of "smarts" can be handled under the hood... Some needed work from Video area contributors! The recording of this talk could interest you given your questions: I hope this helped clarify a few points! |
Note, it might look like the video APIs are a bit opaque, difficult to grasp. This might be due to the fact the documentation effort is standing behind the transformation of the APIs. I believe there are 2 big transformations left:
Then I suppose the video APIs will be more or less as they are expected to be, with minor changes, and documentation can start. Thank you for your patience! |
Thanks @josuah, That makes sense to me! However unless I am reading the code wrongly, the samples appear to have a complete different Note: in my test case I have set: CONFIG_VIDEO_FRAME_WIDTH=320 CONFIG_VIDEO_FRAME_HEIGHT=240
Which is completely opposite.... (I think) |
@josuah and .. I have had some luck with getting it up by defining crop the same as frame... Currently as a test case I am using Nicla Vision, my own sample sketch which sends a captured image over USB. If anyone wishes to play along: Currently the test code is up in: Zephyr updated code is up at: Note: This fork/branch has an attempt at adding a snapshot mode to the video library. At first I was trying to use the RAW image processing sketch that is part of the Arduino library, but it This was a capture with the Nicla Vision pointed at my IPAD which I was displaying a picture.... Probably will now experiment more with a snapshot mode, and with that hopefully allow the Nicla vision to capture |
Replaced by #93797 |
As I mentioned in issue #91968,
It would be great if the camera drivers would support a larger number of resolutions. For example those resolutions of the displays that they wish to display an image on. Potentially in both landscape and portrait mode.
So far I have added one the works in landscape mode for ILI948x and ST7796 displays as well
as a QVGA in portrait mode, which should be good for ILI9341 and some ST7789 displays.
If this looks like a valid approach will also add in GIGA Display Adapter size, and maybe and add the duplicate items to the FORMAT_CAP list for YUYV mode.
Note: the other than add the items to the list, I just added code to set resolution which computes the c_ratio/r_ratio chooses the smaller of the two to use.
Could probably do several of the others n the switch statement.
Example: shows GC2145 image shown on an ST7796 display on an Arduino GIGA using the Arduino wrapper.