[Spice-devel] [PATCH spice-server] video-stream: Document the usage of RedUpgradeItem structure

[Uri Lublin]

On 03/18/2018 12:09 PM, Frediano Ziglio wrote:
>>
>> On 03/13/2018 06:28 PM, Jonathon Jongsma wrote:
>>> Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
>>>
>>>
>>> On Fri, 2018-03-09 at 18:00 +0000, Frediano Ziglio wrote:
>>>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>>>> ---
>>>>    server/video-stream.h | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/server/video-stream.h b/server/video-stream.h
>>>> index cf912152..e1bd0d5a 100644
>>>> --- a/server/video-stream.h
>>>> +++ b/server/video-stream.h
>>>> @@ -45,6 +45,8 @@
>>>>    
>>>>    typedef struct VideoStream VideoStream;
>>>>    
>>>> +/* This item is used to send a full quality image of the area where
>>>> the stream was.
>>>> + * This to avoid the artifacts due to the lossless compression. */
>>
>> Hi,
>>
>> This is not clear to me.
>> With lossless compression there should be no artifacts.
>>
>> Maybe replace with
>> "A lossless image is sent to avoid video stream artifacts"
>>
>> Uri.
>>
> 
> Was/is:
> 
> /* This item is used to send a full quality image of the area where the stream was.
>   * This to avoid the artifacts due to the lossless compression. */
> 
> maybe:
> 
> /* This item is used to send a full quality image (lossless) of the area where the stream was.
>  * This to avoid the artifacts due to the lossy compression. */

looks good to me.

> 
> I think the "lossless" in the old sentence was wrong (after reading again).

I agree.

> I think that "lossless image" is syntactically wrong, is a compression/encoding that is lossy/lossless.

I understand it as "an image that is compressed with a lossless
compression/encoding". I think it's fine to use this term.

Uri.

> 
>>>>    typedef struct RedUpgradeItem {
>>>>        RedPipeItem base;
>>>>        Drawable *drawable;
> 
> Frediano
>