Re: [Spice-devel] [PATCH v4 06/17] sound: Implements config_socket RedChannel callback
> > On Mon, Dec 05, 2016 at 09:16:45AM -0500, Frediano Ziglio wrote: > > > > > > On Thu, Dec 01, 2016 at 03:43:01PM -0600, Jonathon Jongsma wrote: > > > > On Thu, 2016-12-01 at 11:24 +, Frediano Ziglio wrote: > > > > > This code is the same inside __new_channel but will set the > > > > > RedsStream from RedChannel. > > > > > > > > Shouldn't this code be removed from __new_channel() then? > > > > > > I agree that this would make things it more obvious what's going on by > > > looking at the diff. Unless __new_channel is still used for now, and > > > removing this breaks things. > > > > > > Christophe > > > > > > > > > > Should I merge into the GObject patch then? > > I don't know, would it make sense to remove the code from > __new_channel() in this patch, or is this going to break things? > > Christophe > Removing these settings from __new_channel should not break code but changes timing/delays. If in the future you'll like to do some test (like git blame) specific to delays could be that you stop at this commit as causes regressions. Merging on the GObject patch, although it's a big patch, will avoid the regression and get the diff that looks as code move instead of one patch that show code add (this) and the following showing code removal (the GObject one). Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 06/17] sound: Implements config_socket RedChannel callback
On Mon, Dec 05, 2016 at 09:16:45AM -0500, Frediano Ziglio wrote: > > > > On Thu, Dec 01, 2016 at 03:43:01PM -0600, Jonathon Jongsma wrote: > > > On Thu, 2016-12-01 at 11:24 +, Frediano Ziglio wrote: > > > > This code is the same inside __new_channel but will set the > > > > RedsStream from RedChannel. > > > > > > Shouldn't this code be removed from __new_channel() then? > > > > I agree that this would make things it more obvious what's going on by > > looking at the diff. Unless __new_channel is still used for now, and > > removing this breaks things. > > > > Christophe > > > > > > Should I merge into the GObject patch then? I don't know, would it make sense to remove the code from __new_channel() in this patch, or is this going to break things? Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 06/17] sound: Implements config_socket RedChannel callback
> > On Thu, Dec 01, 2016 at 03:43:01PM -0600, Jonathon Jongsma wrote: > > On Thu, 2016-12-01 at 11:24 +, Frediano Ziglio wrote: > > > This code is the same inside __new_channel but will set the > > > RedsStream from RedChannel. > > > > Shouldn't this code be removed from __new_channel() then? > > I agree that this would make things it more obvious what's going on by > looking at the diff. Unless __new_channel is still used for now, and > removing this breaks things. > > Christophe > > Should I merge into the GObject patch then? Frediano ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 06/17] sound: Implements config_socket RedChannel callback
On Thu, Dec 01, 2016 at 03:43:01PM -0600, Jonathon Jongsma wrote: > On Thu, 2016-12-01 at 11:24 +, Frediano Ziglio wrote: > > This code is the same inside __new_channel but will set the > > RedsStream from RedChannel. > > Shouldn't this code be removed from __new_channel() then? I agree that this would make things it more obvious what's going on by looking at the diff. Unless __new_channel is still used for now, and removing this breaks things. Christophe signature.asc Description: PGP signature ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 06/17] sound: Implements config_socket RedChannel callback
>
> On Thu, 2016-12-01 at 11:24 +, Frediano Ziglio wrote:
> > This code is the same inside __new_channel but will set the
> > RedsStream from RedChannel.
>
> Shouldn't this code be removed from __new_channel() then?
>
I'd prefer to keep the functionality and remove when the
feature is replaced correctly to avoid any possible git
blame problem.
Frediano
>
>
> >
> > Signed-off-by: Frediano Ziglio
> > ---
> > server/sound.c | 46 +-
> > 1 file changed, 45 insertions(+), 1 deletion(-)
> >
> > diff --git a/server/sound.c b/server/sound.c
> > index 011cff2..bf59789 100644
> > --- a/server/sound.c
> > +++ b/server/sound.c
> > @@ -1001,7 +1001,51 @@ error1:
> >
> > static int snd_channel_config_socket(RedChannelClient *rcc)
> > {
> > -g_assert_not_reached();
> > +int delay_val;
> > +int flags;
> > +#ifdef SO_PRIORITY
> > +int priority;
> > +#endif
> > +int tos;
> > +RedsStream *stream = red_channel_client_get_stream(rcc);
> > +RedClient *red_client = red_channel_client_get_client(rcc);
> > +MainChannelClient *mcc = red_client_get_main(red_client);
> > +
> > +if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
> > +spice_printerr("accept failed, %s", strerror(errno));
> > +return FALSE;
> > +}
> > +
> > +#ifdef SO_PRIORITY
> > +priority = 6;
> > +if (setsockopt(stream->socket, SOL_SOCKET, SO_PRIORITY,
> > (void*)&priority,
> > + sizeof(priority)) == -1) {
> > +if (errno != ENOTSUP) {
> > +spice_printerr("setsockopt failed, %s",
> > strerror(errno));
> > +}
> > +}
> > +#endif
> > +
> > +tos = IPTOS_LOWDELAY;
> > +if (setsockopt(stream->socket, IPPROTO_IP, IP_TOS, (void*)&tos,
> > sizeof(tos)) == -1) {
> > +if (errno != ENOTSUP) {
> > +spice_printerr("setsockopt failed, %s",
> > strerror(errno));
> > +}
> > +}
> > +
> > +delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
> > +if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY,
> > &delay_val, sizeof(delay_val)) == -1) {
> > +if (errno != ENOTSUP) {
> > +spice_printerr("setsockopt failed, %s",
> > strerror(errno));
> > +}
> > +}
> > +
> > +if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
> > +spice_printerr("accept failed, %s", strerror(errno));
> > +return FALSE;
> > +}
> > +
> > +return TRUE;
> > }
> >
> > static void snd_channel_on_disconnect(RedChannelClient *rcc)
>
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 06/17] sound: Implements config_socket RedChannel callback
On Thu, 2016-12-01 at 11:24 +, Frediano Ziglio wrote:
> This code is the same inside __new_channel but will set the
> RedsStream from RedChannel.
Shouldn't this code be removed from __new_channel() then?
>
> Signed-off-by: Frediano Ziglio
> ---
> server/sound.c | 46 +-
> 1 file changed, 45 insertions(+), 1 deletion(-)
>
> diff --git a/server/sound.c b/server/sound.c
> index 011cff2..bf59789 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1001,7 +1001,51 @@ error1:
>
> static int snd_channel_config_socket(RedChannelClient *rcc)
> {
> -g_assert_not_reached();
> +int delay_val;
> +int flags;
> +#ifdef SO_PRIORITY
> +int priority;
> +#endif
> +int tos;
> +RedsStream *stream = red_channel_client_get_stream(rcc);
> +RedClient *red_client = red_channel_client_get_client(rcc);
> +MainChannelClient *mcc = red_client_get_main(red_client);
> +
> +if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
> +spice_printerr("accept failed, %s", strerror(errno));
> +return FALSE;
> +}
> +
> +#ifdef SO_PRIORITY
> +priority = 6;
> +if (setsockopt(stream->socket, SOL_SOCKET, SO_PRIORITY,
> (void*)&priority,
> + sizeof(priority)) == -1) {
> +if (errno != ENOTSUP) {
> +spice_printerr("setsockopt failed, %s",
> strerror(errno));
> +}
> +}
> +#endif
> +
> +tos = IPTOS_LOWDELAY;
> +if (setsockopt(stream->socket, IPPROTO_IP, IP_TOS, (void*)&tos,
> sizeof(tos)) == -1) {
> +if (errno != ENOTSUP) {
> +spice_printerr("setsockopt failed, %s",
> strerror(errno));
> +}
> +}
> +
> +delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
> +if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY,
> &delay_val, sizeof(delay_val)) == -1) {
> +if (errno != ENOTSUP) {
> +spice_printerr("setsockopt failed, %s",
> strerror(errno));
> +}
> +}
> +
> +if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
> +spice_printerr("accept failed, %s", strerror(errno));
> +return FALSE;
> +}
> +
> +return TRUE;
> }
>
> static void snd_channel_on_disconnect(RedChannelClient *rcc)
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 06/17] sound: Implements config_socket RedChannel callback
This code is the same inside __new_channel but will set the
RedsStream from RedChannel.
Signed-off-by: Frediano Ziglio
---
server/sound.c | 46 +-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/server/sound.c b/server/sound.c
index 011cff2..bf59789 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1001,7 +1001,51 @@ error1:
static int snd_channel_config_socket(RedChannelClient *rcc)
{
-g_assert_not_reached();
+int delay_val;
+int flags;
+#ifdef SO_PRIORITY
+int priority;
+#endif
+int tos;
+RedsStream *stream = red_channel_client_get_stream(rcc);
+RedClient *red_client = red_channel_client_get_client(rcc);
+MainChannelClient *mcc = red_client_get_main(red_client);
+
+if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
+spice_printerr("accept failed, %s", strerror(errno));
+return FALSE;
+}
+
+#ifdef SO_PRIORITY
+priority = 6;
+if (setsockopt(stream->socket, SOL_SOCKET, SO_PRIORITY, (void*)&priority,
+ sizeof(priority)) == -1) {
+if (errno != ENOTSUP) {
+spice_printerr("setsockopt failed, %s", strerror(errno));
+}
+}
+#endif
+
+tos = IPTOS_LOWDELAY;
+if (setsockopt(stream->socket, IPPROTO_IP, IP_TOS, (void*)&tos,
sizeof(tos)) == -1) {
+if (errno != ENOTSUP) {
+spice_printerr("setsockopt failed, %s", strerror(errno));
+}
+}
+
+delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
+if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val,
sizeof(delay_val)) == -1) {
+if (errno != ENOTSUP) {
+spice_printerr("setsockopt failed, %s", strerror(errno));
+}
+}
+
+if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
+spice_printerr("accept failed, %s", strerror(errno));
+return FALSE;
+}
+
+return TRUE;
}
static void snd_channel_on_disconnect(RedChannelClient *rcc)
--
git-series 0.9.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH v4 06/17] sound: Implements config_socket RedChannel callback
This code is the same inside __new_channel but will set the
RedsStream from RedChannel.
Signed-off-by: Frediano Ziglio
---
server/sound.c | 46 +-
1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/server/sound.c b/server/sound.c
index 011cff2..bf59789 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -1001,7 +1001,51 @@ error1:
static int snd_channel_config_socket(RedChannelClient *rcc)
{
-g_assert_not_reached();
+int delay_val;
+int flags;
+#ifdef SO_PRIORITY
+int priority;
+#endif
+int tos;
+RedsStream *stream = red_channel_client_get_stream(rcc);
+RedClient *red_client = red_channel_client_get_client(rcc);
+MainChannelClient *mcc = red_client_get_main(red_client);
+
+if ((flags = fcntl(stream->socket, F_GETFL)) == -1) {
+spice_printerr("accept failed, %s", strerror(errno));
+return FALSE;
+}
+
+#ifdef SO_PRIORITY
+priority = 6;
+if (setsockopt(stream->socket, SOL_SOCKET, SO_PRIORITY, (void*)&priority,
+ sizeof(priority)) == -1) {
+if (errno != ENOTSUP) {
+spice_printerr("setsockopt failed, %s", strerror(errno));
+}
+}
+#endif
+
+tos = IPTOS_LOWDELAY;
+if (setsockopt(stream->socket, IPPROTO_IP, IP_TOS, (void*)&tos,
sizeof(tos)) == -1) {
+if (errno != ENOTSUP) {
+spice_printerr("setsockopt failed, %s", strerror(errno));
+}
+}
+
+delay_val = main_channel_client_is_low_bandwidth(mcc) ? 0 : 1;
+if (setsockopt(stream->socket, IPPROTO_TCP, TCP_NODELAY, &delay_val,
sizeof(delay_val)) == -1) {
+if (errno != ENOTSUP) {
+spice_printerr("setsockopt failed, %s", strerror(errno));
+}
+}
+
+if (fcntl(stream->socket, F_SETFL, flags | O_NONBLOCK) == -1) {
+spice_printerr("accept failed, %s", strerror(errno));
+return FALSE;
+}
+
+return TRUE;
}
static void snd_channel_on_disconnect(RedChannelClient *rcc)
--
git-series 0.9.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
