Re: [RFC PATCH spice-server v3 10/20] stream-channel: Allows to register callback to get new stream request

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 2017-08-25 at 05:39 -0400, Frediano Ziglio wrote:
> > 
> > I had a similar comment in a different patch during the last
> > version of
> > the series, but I personally would prefer a signal to handle this
> > situation. For example, StreamChannel could have a "supported-
> > codecs"
> > property, Then when a new client connected, or a client
> > disconnected,
> > it would update this property. The StreamDevice would connect to
> > the
> > "notify::supported-codecs" signal, and then it would start and stop
> > the
> > stream as necessary. I think that encapsulates the logic of
> > starting
> > and stopping the stream a little bit better within the device class
> > as
> > well.
> > 
> > After writing the above paragraph, I did a little more
> > investigation
> > and I noticed that DisplayChannel already has a similar design --
> > it
> > has a "video-codecs" property.
> > 
> > Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> > 
> 
> As long as I can have type safety, static binding and
> I don't have to write too much I'm not against properties
> and signal.
> 
> As far as I know this is not possible with GObject,
> at least not in the current C implementation.
> 
> Frediano

Yes, there is a tradeoff between type-safety and convenience when
you're using C. But we already have plenty of signals and properties in
other parts of the code. Are you suggesting that we should not have any
of these in the code because they're not fully type-safe? And if not:
why should we treat this class differently and refuse to use them here
when we already use them (for good reasons) elsewhere?

> 
> > 
> > 
> > On Wed, 2017-08-23 at 10:14 +0100, Frediano Ziglio wrote:
> > > The channel needs to communicate when it receive a new
> > > stream request from the guest.
> > > 
> > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > ---
> > >  server/stream-channel.c | 12 ++++++++++++
> > >  server/stream-channel.h |  6 ++++++
> > >  2 files changed, 18 insertions(+)
> > > 
> > > diff --git a/server/stream-channel.c b/server/stream-channel.c
> > > index 08c51272..a89a8f8e 100644
> > > --- a/server/stream-channel.c
> > > +++ b/server/stream-channel.c
> > > @@ -68,6 +68,10 @@ struct StreamChannel {
> > >      int stream_id;
> > >      /* size of the current video stream */
> > >      unsigned width, height;
> > > +
> > > +    /* callback to notify when a stream should be started or
> > > stopped
> > > */
> > > +    stream_channel_start_proc start_cb;
> > > +    void *start_opaque;
> > >  };
> > >  
> > >  struct StreamChannelClass {
> > > @@ -383,3 +387,11 @@ stream_channel_send_data(StreamChannel
> > > *channel,
> > > const void *data, size_t size,
> > >      red_channel_pipes_new_add(red_channel, pipe_item_new_ref,
> > > item);
> > >      red_pipe_item_unref(&item->base);
> > >  }
> > > +
> > > +void
> > > +stream_channel_register_start_cb(StreamChannel *channel,
> > > +                                 stream_channel_start_proc cb,
> > > void
> > > *opaque)
> > > +{
> > > +    channel->start_cb = cb;
> > > +    channel->start_opaque = opaque;
> > > +}
> > > diff --git a/server/stream-channel.h b/server/stream-channel.h
> > > index 6257f806..37b9e327 100644
> > > --- a/server/stream-channel.h
> > > +++ b/server/stream-channel.h
> > > @@ -49,6 +49,7 @@ GType stream_channel_get_type(void)
> > > G_GNUC_CONST;
> > >  StreamChannel* stream_channel_new(RedsState *server);
> > >  
> > >  struct StreamMsgFormat;
> > > +struct StreamMsgStartStop;
> > >  
> > >  void stream_channel_change_format(StreamChannel *channel,
> > >                                    const struct StreamMsgFormat
> > > *fmt);
> > > @@ -56,6 +57,11 @@ void stream_channel_send_data(StreamChannel
> > > *channel,
> > >                                const void *data, size_t size,
> > >                                uint32_t mm_time);
> > >  
> > > +typedef void (*stream_channel_start_proc)(void *opaque, struct
> > > StreamMsgStartStop *start,
> > > +                                          StreamChannel
> > > *channel);
> > > +void stream_channel_register_start_cb(StreamChannel *channel,
> > > +                                      stream_channel_start_proc
> > > cb,
> > > void *opaque);
> > > +
> > >  G_END_DECLS
> > >  
> > >  #endif /* STREAM_CHANNEL_H_ */
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]