Add VIDEO_ENCODER related Kconfig & correct STM32N6 VENC enabling via board specific sample config/overlay #97494
Conversation
The stm32n6570_dk device-tree describing the hardware available on this board, the Video ENCoder (VENC) status should be enabled, and its usage should be enabled or not based on the sample / application configuration. Signed-off-by: Alain Volmat <[email protected]>
zephyrbot
added
area: Samples
zephyrbot
requested review from
57300,
FRASTM,
GeorgeCGV,
djiatsaf-st,
erwango,
gautierg-st,
jeremybettis,
josuah,
loicpoulain,
marwaiehm-st,
mathieuchopstm,
nashif,
ngphibang,
nordicjm and
tejlmand
avolmat-st
force-pushed
the
pr-kconfig-video-encoder-addition
branch
from
fe38175 to
b817f02
Compare
drivers/video/Kconfig
Outdated
|
|
||
| config VIDEO_ENCODER_H264 | ||
| bool "H264 video encoder support" | ||
| default n |
There was a problem hiding this comment.
Kconfig symbols are default n "by default", please remove.
Same below
| CONFIG_NET_CONFIG_MY_IPV4_ADDR="192.0.2.1" | ||
|
|
||
| CONFIG_VIDEO=y | ||
| CONFIG_VIDEO_ENCODER=y |
There was a problem hiding this comment.
Note on this:
What happen if CONFIG_VIDEO_ENCODER is selected but neither CONFIG_VIDEO_ENCODER_H264 or CONFIG_VIDEO_ENCODER_JPEG are ?
Will it compile on other boards that don't have such encoders ?
Otherwise, I suggest to move to boards _venc/_jpeg files.
There was a problem hiding this comment.
I was also wondering about removing CONFIG_VIDEO_ENCODER as DT_HAS_CHOSEN(zephyr_videoenc) provides the same information in samples, and might have not be used in drivers either.
There was a problem hiding this comment.
I put the overall CONFIG_VIDEO_ENCODER thinking about the application rather than the driver in fact. Idea behind it was to have a CONFIG_ to enable/disable encoder support from the application, but indeed since we already have DT_HAS_CHOSEN(zephyr_videoenc) for that, this is kind of duplicated in fact. So I will put directly VIDEO_ENCODER_H264 / VIDEO_ENCODER_JPEG below CONFIG_VIDEO in the Kconfig and have only CONFIG_ENCODER_H264 / JPEG set in the corresponding conf file.
drivers/video/Kconfig
Outdated
| menuconfig VIDEO_ENCODER | ||
| bool "Video encoder drivers" | ||
| help | ||
| Enable support for VIDEO encoder. | ||
|
|
||
| if VIDEO_ENCODER | ||
|
|
||
| config VIDEO_ENCODER_H264 | ||
| bool "H264 video encoder support" | ||
| default n |
There was a problem hiding this comment.
I understand that this is needed to enable only part of functionalities of a device, e.g. a HW can do encoding and decoding, and we want only the encoding part.
But what if a HW cannot be split by this functionality concept ? e.g. a HW is enabled and it will do both : encoding H.264 and H.265
There was a problem hiding this comment.
Original requirement was to be able to build a system which has several drivers part of the video subsystem and be able to not build ALL devices (which would be the case if there is only CONFIG_VIDEO) if only a subset of those is needed.
As an example, on the N6, there are DCMIPP / VENC / JPEG, and if we have CONFIG_VIDEO=y this will build both VENC and JPEG by default which might not be what we want and would as well consume potentially useless code size / memory. That is the reason why I added this.
In case of a unique driver exposing several formats, such as H264 / H265, if this can be split within the driver, then it would be possible to condition code on those VIDEO_ENCODER_H264 / VIDEO_ENCODER_H265 / VIDEO_DECODER_xxx, or, if this really can't be split, then have the driver depends on VIDEO_ENCODER_H264 || VIDEO_ENCODER_H265 || ... maybe ?
There was a problem hiding this comment.
Ok, I got it. It's because both VENC and JPEG are enabled by default so it seems adding additional KConfigs is the only way to build them selectively. Make sense.
There was a problem hiding this comment.
Could there be any use-case for &venc { status = "okay"; }; but CONFIG_VIDEO_ENCODER=n? I am wondering if the VIDEO_ENCODER could be dropped or at least not introduced yet.
Maybe it is a matter of Kconfig best practice, though.
There was a problem hiding this comment.
Could there be any use-case for &venc { status = "okay"; }; but CONFIG_VIDEO_ENCODER=n
This is actually the default practice of enabling all devices in board's default configuration (boards///*.dts) and put the control of what will be used at application level (samples/, tests/). Idea, besides HW vs SW configuration, is to have boards compatible with application w/o requiring to provide an overlay for each board:
Boards default HW configuration is supposed to be compatible with compatible applications and each application enables the SW components they need by Kconfig, w/o having to care about each board configuration.
There was a problem hiding this comment.
I think I mistook the practices for the SoC and for the boards. Thanks for the Kconfig/devicetree class!
I was also thinking users could misconfigure prj.conf to enable only VIDEO_ENCODER and forget to enable VIDEO_ENCODER_H264=y in prj.conf. I did not the best approach/practice, so thanks for the hints.
There was a problem hiding this comment.
If I take all comments here, I think I understand that, VIDEO_ENCODER_xxx is necessary of course in order to be have configuration done at application level and avoid necessary overlay per boards.
For the time being since there is nothing VIDEO_ENCODER so we can get rid of it and only introduce VIDEO_ENCODER_H264 / VIDEO_ENCODER_JPEG.
Add video subsystem related Kconfig in order to allow an application to enable encoder support. This is useful for platforms having several video devices available as well as several video encoders, since it allows to only compile / enable part of those devices depending on their kind. Signed-off-by: Alain Volmat <[email protected]>
Make the STM32 VENC driver depends on the VIDEO_ENCODER_H264 in order to be compiled only if VIDEO_ENCODER_H264 is enabled by an application. Signed-off-by: Alain Volmat <[email protected]>
This reverts commit 2654845. The conf file embedded within the snippet are setting sample specific Kconfig, making its usage impossible with other application. Remove the snippet and introduce instead board specific config & overlay including venc file_suffix in order to allow future addition of other encoders available on the same board. Signed-off-by: Alain Volmat <[email protected]>
Add STM32N6570_DK specific conf files and overlays in order to enable the VENC encoder. Signed-off-by: Alain Volmat <[email protected]>
avolmat-st
force-pushed
the
pr-kconfig-video-encoder-addition
branch
from
b817f02 to
7e3f6cb
Compare
|
6 participants
video-stm32-venc snippet has been introduced previously in order to select / enable the VENC, however this is not fully appropriate since it requires as well to set some sample specific CONFIGs, making this unusable via snippet.
The proposed alternative is to introduce generic CONFIG_VIDEO_ENCODER (and sub config) in order to allow application to enable overall feature and have the VENC driver depends on the CONFIG_VIDEO_ENCODER_H264.
Following this, tcpservsink video sample app board specific conf/overlay files are introduced in order to configure the application and enable the VENC.
FILE_SUFFIX=venc must be set during the building in order to pick up the _venc suffixed conf/overlay files since the N6 also have a JPEG encoder which can also be used with the same sample app / on the same board.