-
Notifications
You must be signed in to change notification settings - Fork 7.9k
video: add video compression support to tcpserversink sample #95862
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
base: main
Are you sure you want to change the base?
video: add video compression support to tcpserversink sample #95862
Conversation
Sync with video capture sample. Signed-off-by: Hugues Fruchet <[email protected]>
Add video compression support to lowerize network bandwidth. To visualise camera content on host PC, use GStreamer command line: $> gst-launch-1.0 tcpclientsrc host=<board ip address> port=5000 ! decodebin ! autovideosink sync=false Signed-off-by: Hugues Fruchet <[email protected]>
Add YUV420 semi-planar support (NV12). This is the video encoder prefered pixel format. Signed-off-by: Hugues Fruchet <[email protected]>
Allow to configure the number of allocated capture frames. This allows to make tradeof between framerate versus memory usage. 2 buffers allows to capture while sending data (optimal framerate). 1 buffer allows to reduce memory usage but capture framerate is lower. Signed-off-by: Hugues Fruchet <[email protected]>
Add configuration files for the stm32n6570_dk board. This enables streaming over ethernet of the images captured by MB1854 camera module compressed in 1920x1088 H264 video bitstream. Signed-off-by: Hugues Fruchet <[email protected]>
|
Good idea to split dependencies out of #92884 to help review. Is the current PR expected to be reviewed now? I am asking just because I did not see You may be interested in this PR which introduces H.264 into the Thank you for bringing this forward! |
@@ -0,0 +1,63 @@ | |||
# Copyright (c) 2024 Espressif Systems (Shanghai) Co., Ltd. |
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.
Is this Copyright on purpose ?
struct video_frmival_enum fie; | ||
enum video_buf_type type = VIDEO_BUF_TYPE_OUTPUT; | ||
const struct device *video_dev; | ||
#if (CONFIG_VIDEO_SOURCE_CROP_WIDTH && CONFIG_VIDEO_SOURCE_CROP_HEIGHT) || \ |
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.
Most of the added code seems to be coming from the capture/src/main.c.
@josuah has started a work on PR #96072 which might lead to having most of this code accessible via helper functions, hence avoiding much of duplication code in the sample apps (and possibly test app).
Should we base this as well on this work to avoid duplicating the code ?
return 0; | ||
} | ||
|
||
#if DT_HAS_CHOSEN(zephyr_videoenc) |
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.
I am not sure if this zephyr,videoenc has already been introduced in any other PR but if this is not the case, I think it should be documented and described:
cf doc/build/dts/api/api.rst
struct video_caps caps; | ||
enum video_buf_type type = VIDEO_BUF_TYPE_OUTPUT; | ||
uint32_t size; | ||
if (encoder_dev) |
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.
It seems to me that such if statement will prevent to restart the encoder after the 2nd loop.
From the main function, configure_encoder and stop_encoder are called at the beginning / end of the main loop.
Such if statement will lead to not doing anything regarding to configuration / set_stream after the first run, while the stop_encoder is always calling set_stream(enable=false) at the end of each loop.
LOG_ERR("%s: video device not ready.", encoder_dev->name); | ||
return -1; | ||
} | ||
LOG_INF("Video device: %s", encoder_dev->name); |
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.
Maybe rename "Video device: .." into "Encoder device: " to make it more visible which device is what since I think there must be another Video device for the camera receiver as well.
return ret; | ||
} | ||
|
||
video_enqueue(encoder_dev, (*out)); |
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.
The frame can only be enqueued again after being fully used by the application, aka, after the sendall other nothing prevent the underlying driver to update the buffer while sendall is using those data.
socklen_t client_addr_len = sizeof(client_addr); | ||
struct video_buffer *buffers[2]; | ||
struct video_buffer *vbuf = &(struct video_buffer){}; | ||
struct video_buffer *vbuf_out = &(struct video_buffer){}; |
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 seems to be only used if
#if DT_HAS_CHOSEN(zephyr_videoenc)
#if DT_HAS_CHOSEN(zephyr_videoenc) | ||
encode_frame(vbuf, &vbuf_out); | ||
|
||
printk("\rSending compressed frame %d (size=%d bytes)\n", i++, vbuf_out->bytesused); |
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.
Maybe add the format (here H264) in the print message, based on the CONFIG_...
if (caps.min_line_count == LINE_COUNT_HEIGHT) { | ||
bsize = fmt.pitch * fmt.height; | ||
if (fmt.pixelformat == VIDEO_PIX_FMT_NV12) { | ||
bsize = fmt.width * fmt.height * 3 / 2; |
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.
Could you use video_bits_per_pixel instead of hardcodying the value here ?
help | ||
Pixel format of the video frame. If not set, the default pixel format is used. | ||
|
||
config VIDEO_NUM_FRAMES |
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.
Maybe have the name including buffer or so since this is related to buffering.
This PR adds support of camera streaming with compression of camera frames in video format such as H264 instead of sending big raw uncompressed frames, reducing drastically the network bandwidth needed for such use-case.
This video compression mode is enabled when a video encoder is found in device-tree, such as the VENC video encoder driver of the STM32N6 platform [1].
On :zephyr:board:
stm32n6570_dk
, the MB1854 IMX335 camera module must be plugged in the CSI-2 camera connector. A RJ45 ethernet cable must be plugged in the ethernet CN6 connector.For an optimal image experience, it is adviced to embed the STM32 image signal processing middleware [2].
To build sample: (flash using ST-Link and boot with FSBL [3])
The default configuration allows to capture and stream 1080p camera content from STM32N6 which can be received, decoded and displayed by an host PC using a GStreamer command line:
[1] #92884
[2] https://github.com/stm32-hotspot/zephyr-stm32-mw-isp
[3] https://docs.zephyrproject.org/latest/boards/st/stm32n6570_dk/doc/index.html