[video_thread] Improvements to video_thread performance #1735
Conversation
|
@guidoAI could you please repeat here what functionality is required? |
| @@ -80,6 +80,9 @@ static void *video_thread_function(void *data) | |||
| { | |||
| struct video_config_t *vid = (struct video_config_t *)data; | |||
|
|
|||
| char print_tag[80]; | |||
| sprintf(print_tag, "video_thread-%s", vid->dev_name); | |||
There was a problem hiding this comment.
if you already have a fixed size char array: use snprintf
There was a problem hiding this comment.
Besides the niceness level, it would be great if the priorities can be set between different vision functions. For example, textons should run at 5Hz and optical flow at 30Hz. With a periodic function this was / (is?) possible, but the vision process is a bit differently arranged now, with all vision functions added with cv_add.
There was a problem hiding this comment.
You still have the option to only copy/save the image in the callback from the video_thread and then run the processing of it in another thread or schedule it as you want...
There is no requirement to run the processing in the callback (and hence the video_thread thread) unless you want to pass a modified version on to the next in the pipeline.
There was a problem hiding this comment.
Copying the reference would be fine, but really copying the image costs time. Can the img ref be used safely at the moment even when the processing will end after a new image comes in? Or is the image memory block freed before?
There was a problem hiding this comment.
@guidoAI the image memory is freed after the computer vision pipeline function is finished, i.e. when all cv_functions are executed
sw/airborne/modules/computer_vision/video_thread.c:149
There was a problem hiding this comment.
Difficult problem. In this case optical flow fits perfectly in the normal image pipeline, but I can understand that algorithms that take longer might want to only have a reference to this image.
One solution is to add some kind of variable to the image struct (or list) that indicates for each function that uses the image whether this function still needs the image...
At the moment the image gets read from the camera we can take a new piece of memory when the previous image is not fully processed yet....
Note that this:
- might take a lot of memory (potential problem on the ARDrone).
- is easy to get bugs in (especially with new students who forget to indicate that they are done).
Maybe a circular image buffer could also work (solves the memory problem), but then your functions don't have control over when the image is re-used again.
Anyway, this does not matter for this pull request I think.
|
Can you please also rebase it on master to not have other already merged PRs in here? |
| @@ -9,6 +9,8 @@ | |||
|
|
|||
| - Possibility to save an image(shot) on the internal memory (JPEG, full size, best quality) | |||
| </description> | |||
|
|
|||
| <define name="VIDEO_THREAD_NICE_LEVEL" value="5" description="Nice level for each separate video thread"/> | |||
There was a problem hiding this comment.
Maybe explain what it is, does, and what values are acceptable?
|
This looks good 👍 |
|
After a rebase and change to But for future improvements on individual computer vision functions that should run with higher prio it IMHO makes way more sense to not do those computations in the callback from the video thread, but instead copy the img and run the computation in a separate thread (as I already pointed out above). |
wilcoschoneveld
force-pushed
the
video_thread_priority
branch
from
a076689
to
e86e7ed
Compare
|
For now I did a rebase to master and implemented the feedback |
| @@ -3,11 +3,10 @@ | |||
| <module name="video_thread" dir="computer_vision"> | |||
| <doc> | |||
| <description> | |||
| Read video in a thread. | |||
There was a problem hiding this comment.
First line of the description (until the dot) is used as brief description and displayed in http://docs.paparazziuav.org/latest/onboard_modules.html
The default 'nice level' for the video_threads is 10, but this needs to be lower in order for some critical vision algorithms to work properly and reliably (such as optical flow hover control #1698).
Other possible improvements to video_thread;