[Spice-devel] [spice v15 02/21] streaming: Remove the width/height encode_frame() parameters

[Christophe Fergeau]

Hey,

On Thu, May 26, 2016 at 05:15:56PM +0200, Francois Gouget wrote:
> encode_frame() needs the QXL_DRAW_COPY operation's SpiceCopy.src_area
> field anyway, so the width and height parameters were redundant.
> 
> Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> ---
>  server/dcc-send.c      | 26 ++++++++------------------
>  server/mjpeg-encoder.c |  1 -
>  server/video-encoder.h |  2 +-
>  3 files changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 5171f9a..9ece37e 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -1660,7 +1660,6 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
>      SpiceImage *image;
>      uint32_t frame_mm_time;
>      uint32_t n;
> -    int width, height;
>      int ret;
>  
>      if (!stream) {
> @@ -1675,21 +1674,13 @@ static int red_marshall_stream_data(RedChannelClient *rcc,
>          return FALSE;
>      }
>  
> -    if (drawable->sized_stream) {
> -        if (red_channel_client_test_remote_cap(rcc, SPICE_DISPLAY_CAP_SIZED_STREAM)) {
> -            SpiceRect *src_rect = &drawable->red_drawable->u.copy.src_area;
> -
> -            width = src_rect->right - src_rect->left;
> -            height = src_rect->bottom - src_rect->top;
> -        } else {
> -            return FALSE;
> -        }
> -    } else {
> -        width = stream->width;
> -        height = stream->height;
> +    if (drawable->sized_stream &&
> +        !red_channel_client_test_remote_cap(rcc, SPICE_DISPLAY_CAP_SIZED_STREAM)) {
> +        return FALSE;
>      }

I'm afraid this bit is not going to work as expected with older clients
(ones not supporting SPICE_DISPLAY_CAP_SIZED_STREAM). I think before
that the size of the streamed region was fixed at stream creation time
(display_channel_create_stream) and assume never to change during
streaming. This was changed (several years ago) to be able to change if
needed, this is the 'sized_stream' code path.
If we want to keep supporting the !sized_stream case, I believe we need
both src_area, and the width/height that were used when the stream
started.
I haven't tested the !sized_stream case, nor looked at the exact
history, so maybe what I'm saying is not correct :)

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20160527/661a9ce7/attachment.sig>