-
Couldn't load subscription status.
- Fork 8.1k
video: H.264 video format support #95407
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?
Conversation
|
Hello @thecapn32, and thank you very much for your first pull request to the Zephyr project! |
dcda85f to
ef080c2
Compare
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.
Thanks for this improvement! Timely with the #92884 pull request doing the same thing but with real hardware, as well as every future video device.
Some quick feedback first, I will have to give a more in-depth round later.
| .fmt.width = 640, \ | ||
| .fmt.height = 320, \ | ||
| .fmt.pitch = 0, \ | ||
| .fmt.pixelformat = VIDEO_PIX_FMT_H264, \ |
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 was probably convenient for testing, but it is better to keep it to what it was, and instead use the video_set_format() API from the application to select H.264.
| if (video_bits_per_pixel(fmt.pixelformat) > 0) { | ||
| buffer_size = fmt.pitch * fmt.height; | ||
| } else { | ||
| buffer_size = fmt.width * fmt.height / 10; |
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 is probably going to need some other strategy as hard to predict the compression ratio.
This might as well be something the user decides, depending on what is expected. For instance, buffer_size = fmt.width * fmt.height / CONFIG_VIDEO_MIN_COMPRESSION_RATIO, or even buffer_size = CONFIG_VIDEO_COMPRESSED_BUFFER_SIZE.
| /** | ||
| * H264 without start code | ||
| */ | ||
| #define VIDEO_PIX_FMT_H264_MVC VIDEO_FOURCC('M', '2', '6', '4') | ||
|
|
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 we can keep 3D TV H.264 for another day and not introduce H264_MVC for now? https://en.wikipedia.org/wiki/Multiview_Video_Coding
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.
In order to move forward on PR #92884, I've cherry-picked this commit but without the M264 definition.
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.
Clicking view this file shows something like this:
unsigned char h264_test_data[] = {
0x00, 0x00, 0x00, 0x01, 0x67, 0x42, 0xc0, 0x1e, 0x8c, 0x68, 0x0a, 0x03,
0xdb, 0x01, 0x01, 0xe1, 0x10, 0x8d, 0x40, 0x00, 0x00, 0x00, 0x01, 0x68,
0xce, 0x3c, 0x80, 0x00, 0x00, 0x00, 0x01, 0x65, 0xb8, 0x00, 0x04, 0x08,
0xf8, 0x84, 0x44, 0x44, 0x18, 0x11, 0xd5, 0x93, 0x80, 0xba, 0x90, 0x00,
0x75, 0x5f, 0xe0, 0x01, 0xcb, 0x33, 0x44, 0x51, 0xa0, 0xd4, 0xdd, 0xfe,
...
0xb4, 0x32, 0x5b, 0x5f, 0xf3, 0xc6, 0x2d, 0xc6, 0x1e, 0xb1, 0xfe, 0x87,
0xbc, 0x0d, 0xdb, 0x8a, 0x58, 0x4b, 0xf9, 0xe0, 0x9d, 0x44, 0xe7, 0xaf,
0x08, 0x5b, 0x9e, 0xa0
};
unsigned int h264_test_data_len = 315304;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 it is interesting to include the file as a binary. I think there are some places in Zephyr where this is done and will try to give pointers on how to do this.
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.
Related #42580 (comment)
I think the best approach would be to remove the blob, and add instructions in the sample doc showing how to generate the header from a video file.
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 would also remove the amount of data on the repo.
Gstreamer and FFmpeg can generate files.
@JarmouniA Would it make sense to add these commands to CMake so that the user does not have to manually copy-paste a command in order to build? This would prevent to use it for i.e. CI.
It does not hurt to also document it though.
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.
Be it in the generated .h file, or in the hard-coded .h file, this needs to be declared as const, so that it fits in the boards it is tested on. Currently we run out of RAM:
/opt/toolchains/zephyr-sdk-0.17.2/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/12.2.0/../../../../arm-zephyr-eabi/bin/ld.bfd: zephyr/zephyr_pre0.elf section `datas' will not fit in region `RAM'
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 paste the ffmpeg command you used if you remember it?
I can try adding a bit of CMake here to generate the headers when H.264 is enabled and paste it there.
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.
yes!
To generate the H.264 file
ffmpeg -f lavfi -i testsrc2=duration=5:size=640x480:rate=30 \
-c:v h264 -b:v 500k -an test.h264you can play it via:
ffplay test.h264Convert it to C array:
xxd -i test.h264 > video_h264_test.hThere 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.
For later: an example of how a custom command might be integrated in CMakeLists.txt:
zephyr/samples/subsys/llext/edk/ext1/CMakeLists.txt
Lines 24 to 33 in 0520dfe
| add_custom_command( | |
| OUTPUT | |
| ${PROJECT_BINARY_DIR}/${PROJECT_NAME}.llext | |
| ${PROJECT_BINARY_DIR}/${PROJECT_NAME}.inc | |
| COMMAND ${CMAKE_C_COMPILER} ${COMPILE_OPTIONS_PROP} | |
| -o ${PROJECT_BINARY_DIR}/${PROJECT_NAME}.llext | |
| ${PROJECT_SOURCE_DIR}/src/main.c | |
| COMMAND xxd -ip ${PROJECT_NAME}.llext | |
| ${PROJECT_NAME}.inc | |
| ) |
It seems like xxd is currently accepted as a dependency in Zephyr already, we can use the same approach.
|
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.
Sorry for the long wait!
I think once the big .h is turned into a small line of ffmpeg this can be merged as a first early version that can be improved later.
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.
Be it in the generated .h file, or in the hard-coded .h file, this needs to be declared as const, so that it fits in the boards it is tested on. Currently we run out of RAM:
/opt/toolchains/zephyr-sdk-0.17.2/arm-zephyr-eabi/bin/../lib/gcc/arm-zephyr-eabi/12.2.0/../../../../arm-zephyr-eabi/bin/ld.bfd: zephyr/zephyr_pre0.elf section `datas' will not fit in region `RAM'
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 paste the ffmpeg command you used if you remember it?
I can try adding a bit of CMake here to generate the headers when H.264 is enabled and paste it there.



This is an attempt to support H.264 video compressed format on native_sim.
There is H.264 video provided in C array .h file and what is doing now is read video data from that file and send it over TCP. Due to this the frame rates are wrong.
This works needs to be done: