drivers: video: introduce VIDEO_FOURCC_TO_STR #86199
Conversation
josuah
force-pushed
the
pr-video-string-format
branch
from
25ff916 to
4f51883
Compare
|
Wrapping it in parenthesis seemed necessary in some cases. |
josuah
added
the
Trivial
Yep, totally the right approach FWIW :) https://docs.zephyrproject.org/latest/contribute/coding_guidelines/index.html#:~:text=Expressions%20resulting%20from%20the%20expansion%20of%20macro%20parameters%20shall%20be%20enclosed%20in%20parentheses |
include/zephyr/drivers/video.h
Outdated
| * @param fcc 32-bit integer to be converted | ||
| * @return Four-character-string built out of this fourcc. | ||
| */ | ||
| #define VIDEO_FOURCC_TO_STR(u) ((char []){(u) >> 0, (u) >> 8, (u) >> 16, (u) >> 24, '\0'}) |
There was a problem hiding this comment.
don't each byte need to explicitly be cast to char? probably warning-prone otherwise?
also not sure about the naming u, and I think you might face endianness issues?
#define VIDEO_FOURCC_TO_STR(fourcc) ((char []){ \
(char)((fourcc) & 0xFF), \
(char)(((fourcc) >> 8) & 0xFF), \
(char)(((fourcc) >> 16) & 0xFF), \
(char)(((fourcc) >> 24) & 0xFF), \
'\0' \
})There was a problem hiding this comment.
you might face endianness issues
The pixelformat used everywhere in video.h is what FOURCC are used for, and is always in the native endianess of the CPU. That can lead to surprises if logging something directly from the network, I added a comment.
Thank you for the improvement and reminding that clarity and correctness are better than compactness.
There was a problem hiding this comment.
- little/big-endian (LSB/MSB first)
- native-endian (what the CPU has)
- confused-endian (the opposite) like FOURCCs
kartben
removed
the
Trivial
josuah
force-pushed
the
pr-video-string-format
branch
from
4f51883 to
2d55aa1
Compare
This adds a macro to generate a C99 compound literal string, which allow
to use it in debug log messages, such as:
LOG_DBG("The pixel format is '%s'", VIDEO_FOURCC_STR(fmt->pixelformat));
Signed-off-by: Josuah Demangeon <[email protected]>
josuah
force-pushed
the
pr-video-string-format
branch
from
2d55aa1 to
24b7230
Compare
ngphibang
left a comment
ngphibang
left a comment
There was a problem hiding this comment.
I think the PR should also include changes that uses this macro, at least in the video samples. This avoids adding additional PRs later for it (and avoid adding things that has no usage). Otherwise, LGTM
A macro introduced allows to log four-character-codes (FOURCC) as one string instead of enumerating each individual character. This saves a bit of room in the code and is less error-prone. Signed-off-by: Josuah Demangeon <[email protected]>
4 participants
This adds a macro to generate a C99 compound literal string, which allow to use it in debug log messages, such as:
LOG_DBG("The pixel format is '%s'", VIDEO_FOURCC_STR(fmt->pixelformat));The previous proposal was more awkward:
printf("video format: " PRIvfmt, PRIvfmt_arg(&fmt));