On 10/25/19 9:43 AM, Patrice CHOTARD wrote:
> Hi Heinrich
>
> On 10/24/19 10:43 PM, Heinrich Schuchardt wrote:
>> On 10/24/19 4:05 PM, Patrice CHOTARD wrote:
>>> Hi Heinrich, all
>>>
>>> On 10/7/19 7:34 PM, Heinrich Schuchardt wrote:
>>>> On 10/7/19 3:29 PM, Yannick Fertré wrote:
>>>>> If the size of the bitmap is bigger than the size of
>>>>> the panel then errors appear when calculating axis alignment
>>>>> and the copy of bitmap is done outside of framebuffer.
>>>>>
>>>>> Signed-off-by: Yannick Fertré <yannick.fer...@st.com>
>>>>> ---
>>>>> drivers/video/video_bmp.c | 7 +++++++
>>>>> 1 file changed, 7 insertions(+)
>>>>>
>>>>> diff --git a/drivers/video/video_bmp.c b/drivers/video/video_bmp.c
>>>>> index 193f37d..4af1fb4 100644
>>>>> --- a/drivers/video/video_bmp.c
>>>>> +++ b/drivers/video/video_bmp.c
>>>>> @@ -249,6 +249,13 @@ int video_bmp_display(struct udevice *dev, ulong
>>>>> bmp_image, int x, int y,
>>>>>
>>>>> padded_width = (width & 0x3 ? (width & ~0x3) + 4 : width);
>>>>>
>>>>> + /* check if picture size exceeds panel size */
>>>>> + if ((pwidth < width) || (priv->ysize < height)) {
>>>>> + printf("Error: BMP size %d x %d is bigger than panel size %d x
>>>>> %d\n",
>>>>> + (int)width, (int)height, priv->xsize, priv->ysize);
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> if (align) {
>>>>> video_splash_align_axis(&x, priv->xsize, width);
>>>>> video_splash_align_axis(&y, priv->ysize, height);
>>>>
>>>> This is followed by:
>>>>
>>>> if ((x + width) > pwidth)
>>>> width = pwidth - x;
>>>> if ((y + height) > priv->ysize)
>>>> height = priv->ysize - y;
>>>>
>>>> These lines will clip a bitmap that given the left upper corner x, y
>>>> does not fit onto the screen.
>>>>
>>>> So isn't this patch superfluous?
>>>>
>>>> What is missing are checks for x and y.
>>>>
>>>> For testing I have used qemu_x86 and added
>>>>
>>>> #define CONFIG_BMP_24BPP y
>>>>
>>>> to the top of drivers/video/video_bmp.c and loaded a 24bit bitmap.
>>>>
>>>> Clipping works as expected. But passing a value of x exceeding the
>>>> screen width lead to a crash.
>>>>
>>>> What I really dislike in the existing coding is that CONFIG_BMP_*BPP is
>>>> defined in includes instead of using Kconfig but that is a different story.
>>>>
>>>> Best regards
>>>>
>>>> Heinrich
>>>
>>> For information all this series patches have been applied on u-boot/master
>>> excepts this one.
>>>
>>> Unfortunately, without this patch, stm32f746-discovery board doesn't boot
>>> anymore.
>>
>> I still do not understand why this patch is needed.
>>
>> Could you, please, try to analyze why the board does not boot.
>>
>> What is wrong with the existing code for clipping?
>> Or is the non-booting just coincidence but the bug is somewhere else?
>>
>> What are the values of the parameters passed to video_bmp_display()?
>> Which bitmap file are you using?
>> What is the size of the display?
>
>
> To sum-up, on all STM32 boards, the same BMP splashcreen is used.
>
> In the particular case of STM32F746-Disco board, the panel size is smaller
> then the BMP size.
>
> So, some size check must be done to avoid overflow when writing inside the
> framebuffer.
That is why we have lines to clip images:
if ((x + width) > pwidth)
width = pwidth - x;
if ((y + height) > priv->ysize)
height = priv->ysize - y;
Why is this not working in you case?
Best regards
Heinrich
>
> If needed, Yannick, which is multimedia expert, can give you more precise
> details.
>
> Thanks
>
> Patrice
>
>
>>
>> Best regards
>>
>> Heinrich
>>
>>>
>>> Heinrich, could this patch be merged, waiting for a clean patch from
>>> Yannick ?
>>>
>>> Regards
>>>
>>> Patrice
>>>
>>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
