[vbox-dev] [PATCH] VirtualBox Additions DIB Patch

[Klaus Espenlaub]

Hi Sergey,

On 12.09.2013 13:39, Sergey Staroletov wrote:
> Hello, did anybody found our patch useful? Why not to include it to the
> trunk?

The patch would be trivial to integrate if its code wouldn't contradict 
its comments and would be more complete/correct :)

For example the comments hint that BI_BITFIELDS are not handled, but the 
check in the code accepts both BI_RGB and BI_BITFIELDS and proceeds with 
them. For BI_BITFIELDS it will not cause trouble if height is positive 
(or if it is already DIBv1 - but still the code has to be more 
consistent. I suspect it shreds the data in the code path which changes 
orientation or header size.

Also, it's unacceptable that the code malfunctions (for upside down 
images or if the code recreates the header which actually shouldn't ever 
happen) if there is a color table (8bpp or less) or is BI_BITFIELDS. It 
silently drops the color table, making the DIB violate the spec. The 
result will be unpredictable, ranging from image corruption to crashes. 
Depending on what images the user is dealing with I have the impression 
that there can be more crashes than before.

Can you provide a patch with those issues addressed? Would speed up 
integration, as we'd first have to dig into the subtleties of the DIB 
format before we can efficiently bring your contribution to production 
quality.

Thanks for trying to improve VirtualBox, we appreciate any contributions,

Klaus

>
> Thanks, Sergey
>
> On 19.06.2013 10:49, Sergey Staroletov wrote:
>> Hi François,
>>
>> On 19.06.2013 09:33, François Revol wrote:
>>> Thanks for working on this, I knew it wasn't fully working when I
>>> wrote
>>> the initial code, but it worked for me, and I don't have a Mac
>>> anymore
>>> anyway.
>>
>>> Of course best would be to settle on a really standardized (PNG?)
>>> format
>>> for interchange instead of a "mostly documented" one, but well...
>>>
>>
>> Yes may be it a better solution, but we worked on it for one client
>> and we can release only this patch as he wants it to be opensource.
>>
>>>>
>>>> We modified VBoxClipboard.cpp. We were used VirtualBox-4.2.12
>>>> guests for
>>>> tests.
>>>
>>> It's usually best to submit diffs in unified format (diff -u) so
>>> that
>>> they contain context lines around for patch to make sure it doesn't
>>> break anything.
>>>
>>> Also, your patch has much higher chances of getting in if you diff
>>> against the current svn trunk, in which case "svn diff" will produce
>>> the
>>> unified format automatically.
>>
>>
>> no problem, here is diff -u for the current trunk.
>>
>>
>>> François.
>
> ----------------------------------
> Sergey Staroletov
> Senior UNIX C++ developer
> Sibers (HireRussians/former Key-Soft Ltd.)
> E-mail: sergey.staroletov at sibers.com