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

[Sergey Staroletov]

Hello, have anyone looked to this patch?:) Or it was interesting to us 
only?
Our client wants it to be opensource.


On 30.09.2013 11:53, Sergey Staroletov wrote:
> Hi, we improved the patch a little bit, please see comments.
>
>
> On 16.09.2013 14:12, Klaus Espenlaub wrote:
>> 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.
>>
>
> Now the code matches this comment, images with BI_BITFIELDS are
> passing and aren't processed with our function.
> Also we are skipping compressed images, images with palette and
> images with null height or width.
>
>
>> 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.
>
> 8bpp checking was in the old code. Now all images with palette are
> skipping, that's why copying is
> processing right. If we required to flip image with DIB header
> version 1, we don't change the header but just copy it, and put the
> flipped data behind it.
>
>
>> 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