[Spice-devel,client,v2,1/3] streaming: Modify display_update_stream_report() to take a SpiceChannel

Submitted by Francois Gouget on Aug. 18, 2016, 3:57 p.m.

Details

Message ID 567f4f450bb929268e68d99fc9e1cd3db98e87a1.1471535534.git.fgouget@free.fr
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Francois Gouget Aug. 18, 2016, 3:57 p.m.
This makes it consistent with the other channel-display functions.

Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
---

This patch makes sense on its own and can be applied even if the 
other two in the series are not.


 src/channel-display.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/channel-display.c b/src/channel-display.c
index 709b3d2..54df9c0 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -1221,10 +1221,10 @@  void stream_display_frame(display_stream *st, SpiceMsgIn *frame_msg,
  * if the report window is bigger */
 #define STREAM_REPORT_DROP_SEQ_LEN_LIMIT 3
 
-static void display_update_stream_report(SpiceDisplayChannel *channel, uint32_t stream_id,
+static void display_update_stream_report(SpiceChannel *channel, uint32_t stream_id,
                                          uint32_t frame_time, int32_t latency)
 {
-    SpiceDisplayChannelPrivate *c = channel->priv;
+    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
     display_stream *st;
     guint64 now;
 
@@ -1232,7 +1232,7 @@  static void display_update_stream_report(SpiceDisplayChannel *channel, uint32_t
     g_return_if_fail(c->streams != NULL);
     g_return_if_fail(c->nstreams > stream_id);
 
-    st = channel->priv->streams[stream_id];
+    st = c->streams[stream_id];
     g_return_if_fail(st != NULL);
 
     if (!st->report_is_active) {
@@ -1257,7 +1257,7 @@  static void display_update_stream_report(SpiceDisplayChannel *channel, uint32_t
         now - st->report_start_time >= st->report_timeout ||
         st->report_drops_seq_len >= STREAM_REPORT_DROP_SEQ_LEN_LIMIT) {
         SpiceMsgcDisplayStreamReport report;
-        SpiceSession *session = spice_channel_get_session(SPICE_CHANNEL(channel));
+        SpiceSession *session = spice_channel_get_session(channel);
         SpiceMsgOut *msg;
 
         report.stream_id = stream_id;
@@ -1273,7 +1273,7 @@  static void display_update_stream_report(SpiceDisplayChannel *channel, uint32_t
             report.audio_delay = UINT_MAX;
         }
 
-        msg = spice_msg_out_new(SPICE_CHANNEL(channel), SPICE_MSGC_DISPLAY_STREAM_REPORT);
+        msg = spice_msg_out_new(channel, SPICE_MSGC_DISPLAY_STREAM_REPORT);
         msg->marshallers->msgc_display_stream_report(msg->marshaller, &report);
         spice_msg_out_send(msg);
 
@@ -1403,7 +1403,7 @@  static void display_handle_stream_data(SpiceChannel *channel, SpiceMsgIn *in)
      */
     st->video_decoder->queue_frame(st->video_decoder, in,  latency);
     if (c->enable_adaptive_streaming) {
-        display_update_stream_report(SPICE_DISPLAY_CHANNEL(channel), op->id,
+        display_update_stream_report(channel, op->id,
                                      op->multi_media_time, latency);
         if (st->playback_sync_drops_seq_len >= STREAM_PLAYBACK_SYNC_DROP_SEQ_LEN_LIMIT) {
             spice_session_sync_playback_latency(spice_channel_get_session(channel));

Comments

> 
> This makes it consistent with the other channel-display functions.
> 
> Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
> ---
> 
> This patch makes sense on its own and can be applied even if the
> other two in the series are not.
> 
> 

I prefer the actual version.

Frediano

>  src/channel-display.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 709b3d2..54df9c0 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -1221,10 +1221,10 @@ void stream_display_frame(display_stream *st,
> SpiceMsgIn *frame_msg,
>   * if the report window is bigger */
>  #define STREAM_REPORT_DROP_SEQ_LEN_LIMIT 3
>  
> -static void display_update_stream_report(SpiceDisplayChannel *channel,
> uint32_t stream_id,
> +static void display_update_stream_report(SpiceChannel *channel, uint32_t
> stream_id,
>                                           uint32_t frame_time, int32_t
>                                           latency)
>  {
> -    SpiceDisplayChannelPrivate *c = channel->priv;
> +    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
>      display_stream *st;
>      guint64 now;
>  
> @@ -1232,7 +1232,7 @@ static void
> display_update_stream_report(SpiceDisplayChannel *channel, uint32_t
>      g_return_if_fail(c->streams != NULL);
>      g_return_if_fail(c->nstreams > stream_id);
>  
> -    st = channel->priv->streams[stream_id];
> +    st = c->streams[stream_id];
>      g_return_if_fail(st != NULL);
>  
>      if (!st->report_is_active) {
> @@ -1257,7 +1257,7 @@ static void
> display_update_stream_report(SpiceDisplayChannel *channel, uint32_t
>          now - st->report_start_time >= st->report_timeout ||
>          st->report_drops_seq_len >= STREAM_REPORT_DROP_SEQ_LEN_LIMIT) {
>          SpiceMsgcDisplayStreamReport report;
> -        SpiceSession *session =
> spice_channel_get_session(SPICE_CHANNEL(channel));
> +        SpiceSession *session = spice_channel_get_session(channel);
>          SpiceMsgOut *msg;
>  
>          report.stream_id = stream_id;
> @@ -1273,7 +1273,7 @@ static void
> display_update_stream_report(SpiceDisplayChannel *channel, uint32_t
>              report.audio_delay = UINT_MAX;
>          }
>  
> -        msg = spice_msg_out_new(SPICE_CHANNEL(channel),
> SPICE_MSGC_DISPLAY_STREAM_REPORT);
> +        msg = spice_msg_out_new(channel, SPICE_MSGC_DISPLAY_STREAM_REPORT);
>          msg->marshallers->msgc_display_stream_report(msg->marshaller,
>          &report);
>          spice_msg_out_send(msg);
>  
> @@ -1403,7 +1403,7 @@ static void display_handle_stream_data(SpiceChannel
> *channel, SpiceMsgIn *in)
>       */
>      st->video_decoder->queue_frame(st->video_decoder, in,  latency);
>      if (c->enable_adaptive_streaming) {
> -        display_update_stream_report(SPICE_DISPLAY_CHANNEL(channel), op->id,
> +        display_update_stream_report(channel, op->id,
>                                       op->multi_media_time, latency);
>          if (st->playback_sync_drops_seq_len >=
>          STREAM_PLAYBACK_SYNC_DROP_SEQ_LEN_LIMIT) {
>              spice_session_sync_playback_latency(spice_channel_get_session(channel));
On Thu, 18 Aug 2016, Frediano Ziglio wrote:

> > 
> > This makes it consistent with the other channel-display functions.
> > 
> > Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
> > ---
> > 
> > This patch makes sense on its own and can be applied even if the
> > other two in the series are not.
> > 
> > 
> 
> I prefer the actual version.

Why?

IMHO it makes no sense to cast a SpiceChannel* to a DisplayChannel* when 
calling display_update_stream_report() only to cast it back, twice, to a 
SpiceChannel* inside that function.
> 
> On Thu, 18 Aug 2016, Frediano Ziglio wrote:
> 
> > > 
> > > This makes it consistent with the other channel-display functions.
> > > 
> > > Signed-off-by: Francois Gouget <fgouget@codeweavers.com>
> > > ---
> > > 
> > > This patch makes sense on its own and can be applied even if the
> > > other two in the series are not.
> > > 
> > > 
> > 
> > I prefer the actual version.
> 
> Why?
> 
> IMHO it makes no sense to cast a SpiceChannel* to a DisplayChannel* when
> calling display_update_stream_report() only to cast it back, twice, to a
> SpiceChannel* inside that function.
> 

Because an apple is a fruit but a fruit is not an apple.

Frediano