[Spice-devel,client,1/3] streaming: Remove the display_stream dependency on SpiceMsgIn messages

Submitted by Francois Gouget on April 4, 2017, 3:45 p.m.

Details

Message ID 3b4084581acc31a4c45dfe980d665146426ced30.1491320131.git.fgouget@free.fr
State New
Headers show
Series "Abstract video streaming from the network details" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Francois Gouget April 4, 2017, 3:45 p.m.
Code-wise this improves the separation between networking and the video
decoding.
It also makes it easier to reuse the latter should the client one day
receive video streams through other messages.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
---
 src/channel-display-priv.h |  7 +++----
 src/channel-display.c      | 37 ++++++++++---------------------------
 2 files changed, 13 insertions(+), 31 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
index b9c08a35..60cc8efa 100644
--- a/src/channel-display-priv.h
+++ b/src/channel-display-priv.h
@@ -99,12 +99,11 @@  typedef struct drops_sequence_stats {
 } drops_sequence_stats;
 
 struct display_stream {
-    SpiceMsgIn                  *msg_create;
-    SpiceMsgIn                  *msg_clip;
-
     /* from messages */
+    uint32_t flags;
+    SpiceRect dest;
     display_surface             *surface;
-    const SpiceClip             *clip;
+    SpiceClip                   clip;
     QRegion                     region;
     int                         have_region;
 
diff --git a/src/channel-display.c b/src/channel-display.c
index 7a5a23bb..2423fb0e 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1118,11 +1118,11 @@  static void display_update_stream_region(display_stream *st)
 {
     int i;
 
-    switch (st->clip->type) {
+    switch (st->clip.type) {
     case SPICE_CLIP_TYPE_RECTS:
         region_clear(&st->region);
-        for (i = 0; i < st->clip->rects->num_rects; i++) {
-            region_add(&st->region, &st->clip->rects->rects[i]);
+        for (i = 0; i < st->clip.rects->num_rects; i++) {
+            region_add(&st->region, &st->clip.rects->rects[i]);
         }
         st->have_region = true;
         break;
@@ -1191,9 +1191,9 @@  static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
     c->streams[op->id] = g_new0(display_stream, 1);
     st = c->streams[op->id];
 
-    st->msg_create = in;
-    spice_msg_in_ref(in);
-    st->clip = &op->clip;
+    st->flags = op->flags;
+    st->dest = op->dest;
+    st->clip = op->clip;
     st->surface = find_surface(c, op->surface_id);
     st->channel = channel;
     st->drops_seqs_stats_arr = g_array_new(FALSE, FALSE, sizeof(drops_sequence_stats));
@@ -1225,9 +1225,7 @@  static const SpiceRect *stream_get_dest(display_stream *st, SpiceMsgIn *frame_ms
 {
     if (frame_msg == NULL ||
         spice_msg_in_type(frame_msg) != SPICE_MSG_DISPLAY_STREAM_DATA_SIZED) {
-        SpiceMsgDisplayStreamCreate *info = spice_msg_in_parsed(st->msg_create);
-
-        return &info->dest;
+        return &st->dest;
     } else {
         SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(frame_msg);
 
@@ -1236,13 +1234,6 @@  static const SpiceRect *stream_get_dest(display_stream *st, SpiceMsgIn *frame_ms
 
 }
 
-static uint32_t stream_get_flags(display_stream *st)
-{
-    SpiceMsgDisplayStreamCreate *info = spice_msg_in_parsed(st->msg_create);
-
-    return info->flags;
-}
-
 G_GNUC_INTERNAL
 uint32_t spice_msg_in_frame_data(SpiceMsgIn *frame_msg, uint8_t **data)
 {
@@ -1288,7 +1279,7 @@  void stream_display_frame(display_stream *st, SpiceMsgIn *frame_msg,
     dest = stream_get_dest(st, frame_msg);
 
     stride = width * sizeof(uint32_t);
-    if (!(stream_get_flags(st) & SPICE_STREAM_FLAGS_TOP_DOWN)) {
+    if (!(st->flags & SPICE_STREAM_FLAGS_TOP_DOWN)) {
         data += stride * (height - 1);
         stride = -stride;
     }
@@ -1502,12 +1493,8 @@  static void display_handle_stream_clip(SpiceChannel *channel, SpiceMsgIn *in)
     display_stream *st = get_stream_by_id(channel, op->id);
 
     g_return_if_fail(st != NULL);
-    if (st->msg_clip) {
-        spice_msg_in_unref(st->msg_clip);
-    }
-    spice_msg_in_ref(in);
-    st->msg_clip = in;
-    st->clip = &op->clip;
+
+    st->clip = op->clip;
     display_update_stream_region(st);
 }
 
@@ -1561,10 +1548,6 @@  static void destroy_stream(SpiceChannel *channel, int id)
         st->video_decoder->destroy(st->video_decoder);
     }
 
-    if (st->msg_clip)
-        spice_msg_in_unref(st->msg_clip);
-    spice_msg_in_unref(st->msg_create);
-
     g_free(st);
     c->streams[id] = NULL;
 }

Comments

On Tue, Apr 04, 2017 at 05:45:28PM +0200, Francois Gouget wrote:
> Code-wise this improves the separation between networking and the video
> decoding.
> It also makes it easier to reuse the latter should the client one day
> receive video streams through other messages.

Ok, this moves message parsing to display_handle_stream_create() rather
than doing it when needed later on. I'd add this to the commit log to
make it easier to parse the patch :) Makes sense to me, and from a quick
glance, code looked ok.

Christophe

> 
> Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
> ---
>  src/channel-display-priv.h |  7 +++----
>  src/channel-display.c      | 37 ++++++++++---------------------------
>  2 files changed, 13 insertions(+), 31 deletions(-)
> 
> diff --git a/src/channel-display-priv.h b/src/channel-display-priv.h
> index b9c08a35..60cc8efa 100644
> --- a/src/channel-display-priv.h
> +++ b/src/channel-display-priv.h
> @@ -99,12 +99,11 @@ typedef struct drops_sequence_stats {
>  } drops_sequence_stats;
>  
>  struct display_stream {
> -    SpiceMsgIn                  *msg_create;
> -    SpiceMsgIn                  *msg_clip;
> -
>      /* from messages */
> +    uint32_t flags;
> +    SpiceRect dest;
>      display_surface             *surface;
> -    const SpiceClip             *clip;
> +    SpiceClip                   clip;
>      QRegion                     region;
>      int                         have_region;
>  
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 7a5a23bb..2423fb0e 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -1118,11 +1118,11 @@ static void display_update_stream_region(display_stream *st)
>  {
>      int i;
>  
> -    switch (st->clip->type) {
> +    switch (st->clip.type) {
>      case SPICE_CLIP_TYPE_RECTS:
>          region_clear(&st->region);
> -        for (i = 0; i < st->clip->rects->num_rects; i++) {
> -            region_add(&st->region, &st->clip->rects->rects[i]);
> +        for (i = 0; i < st->clip.rects->num_rects; i++) {
> +            region_add(&st->region, &st->clip.rects->rects[i]);
>          }
>          st->have_region = true;
>          break;
> @@ -1191,9 +1191,9 @@ static void display_handle_stream_create(SpiceChannel *channel, SpiceMsgIn *in)
>      c->streams[op->id] = g_new0(display_stream, 1);
>      st = c->streams[op->id];
>  
> -    st->msg_create = in;
> -    spice_msg_in_ref(in);
> -    st->clip = &op->clip;
> +    st->flags = op->flags;
> +    st->dest = op->dest;
> +    st->clip = op->clip;
>      st->surface = find_surface(c, op->surface_id);
>      st->channel = channel;
>      st->drops_seqs_stats_arr = g_array_new(FALSE, FALSE, sizeof(drops_sequence_stats));
> @@ -1225,9 +1225,7 @@ static const SpiceRect *stream_get_dest(display_stream *st, SpiceMsgIn *frame_ms
>  {
>      if (frame_msg == NULL ||
>          spice_msg_in_type(frame_msg) != SPICE_MSG_DISPLAY_STREAM_DATA_SIZED) {
> -        SpiceMsgDisplayStreamCreate *info = spice_msg_in_parsed(st->msg_create);
> -
> -        return &info->dest;
> +        return &st->dest;
>      } else {
>          SpiceMsgDisplayStreamDataSized *op = spice_msg_in_parsed(frame_msg);
>  
> @@ -1236,13 +1234,6 @@ static const SpiceRect *stream_get_dest(display_stream *st, SpiceMsgIn *frame_ms
>  
>  }
>  
> -static uint32_t stream_get_flags(display_stream *st)
> -{
> -    SpiceMsgDisplayStreamCreate *info = spice_msg_in_parsed(st->msg_create);
> -
> -    return info->flags;
> -}
> -
>  G_GNUC_INTERNAL
>  uint32_t spice_msg_in_frame_data(SpiceMsgIn *frame_msg, uint8_t **data)
>  {
> @@ -1288,7 +1279,7 @@ void stream_display_frame(display_stream *st, SpiceMsgIn *frame_msg,
>      dest = stream_get_dest(st, frame_msg);
>  
>      stride = width * sizeof(uint32_t);
> -    if (!(stream_get_flags(st) & SPICE_STREAM_FLAGS_TOP_DOWN)) {
> +    if (!(st->flags & SPICE_STREAM_FLAGS_TOP_DOWN)) {
>          data += stride * (height - 1);
>          stride = -stride;
>      }
> @@ -1502,12 +1493,8 @@ static void display_handle_stream_clip(SpiceChannel *channel, SpiceMsgIn *in)
>      display_stream *st = get_stream_by_id(channel, op->id);
>  
>      g_return_if_fail(st != NULL);
> -    if (st->msg_clip) {
> -        spice_msg_in_unref(st->msg_clip);
> -    }
> -    spice_msg_in_ref(in);
> -    st->msg_clip = in;
> -    st->clip = &op->clip;
> +
> +    st->clip = op->clip;
>      display_update_stream_region(st);
>  }
>  
> @@ -1561,10 +1548,6 @@ static void destroy_stream(SpiceChannel *channel, int id)
>          st->video_decoder->destroy(st->video_decoder);
>      }
>  
> -    if (st->msg_clip)
> -        spice_msg_in_unref(st->msg_clip);
> -    spice_msg_in_unref(st->msg_create);
> -
>      g_free(st);
>      c->streams[id] = NULL;
>  }
> -- 
> 2.11.0
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel