-
Notifications
You must be signed in to change notification settings - Fork 618
os/drivers/lcd/mipi_lcd.c: Add runtime BMP to RGB565 conversion #6990
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
Conversation
|
Add bmp file(16bit) inside resource directory and modify the variable |
8f22a48 to
11f55d9
Compare
|
@abhinav-s235 Is it possible to check converting time for 320 240 and 800 480? |
os/drivers/lcd/mipi_lcd.c
Outdated
|
|
||
| static int lcd_render_bmp(FAR struct lcd_dev_s *dev) | ||
| { | ||
| const char *bmp_filename = "/res/kernel/audio/img_logo_samsung.bmp"; |
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.
Use bmp_filename as function parameter
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.
@seokhun-eom24 , How about making the BMP file path configurable in Kconfig?
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.
@seokhun-eom24 , How about making the BMP file path configurable in Kconfig?
I think it's better to parameterize for function reusability.
Additionally, since the BMP file path needs to be determined dynamically at runtime, we should consider whether Kconfig is more suitable.
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.
bmp_filename has been parameterized
| } | ||
| } | ||
| priv->config->lcd_put_area((u8 *)fullscreen_buffer, 1, 1, CONFIG_LCD_XRES, CONFIG_LCD_YRES); | ||
| fclose(bmp_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.
Also needed mm_free(fullscreen_buffer); in normal exit case.
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 normal exit case when SW_ROTATION is disabled
If I free the buffer the LCD will show some random colours
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.
Oh you're right. I thought lcd_put_area copies the buffer. So it doesn't need buffer any more.
os/drivers/lcd/mipi_lcd.c
Outdated
| if (r_mask == 0) { | ||
| r = 0; | ||
| } else { | ||
| uint8_t r_bits = __builtin_popcount(r_mask); |
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.
Need to check with #ifdef __GNUC__ for __builtin_popcount, __builtin_ctz exists.
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.
added helper functions instead of compiler intrinsics
os/drivers/lcd/mipi_lcd.c
Outdated
| int displayed_img_width = img_height; | ||
| int displayed_img_height = img_width; |
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.
If these variables are unnecessary, remove them and use img_height and img_width instead
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.
changes made as you suggested
|
|
||
| int img_width = info_header.width; | ||
| int img_height = info_header.height; | ||
| // For 16-bit, each pixel is 2 bytes. Padding is to the next 4-byte boundary. |
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.
Can you handle when img_width or img_height is larger than LCD width or height.
goto errout for these cases.
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.
Done
You mean time require to convert bmp file? |
…isplay - This commit introduces a new utility to convert BMP image files to the RGB565 color format and display them on the LCD at runtime. - Supports reading 16-bit files, including handling of `BI_RGB` and `BI_BITFIELDS` compression for 16-bit variants.
11f55d9 to
7de15f4
Compare
If its time then it takes around 44ms for 800*480 |
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.
lcd_render_bmp function should be callable from lcd_init_put_image as you did.
This function must verify the provided file path, return ERROR if the file is missing or an other error occurs, otherwise just render the bmp image to LCD with lcd_put_area
When implementation is completed, change state to mergeable.
BI_RGBandBI_BITFIELDScompression for 16-bit variants.