Re: [Spice-devel] [PATCH spice-gtk 1/4] tests: add spice+unix:// URI checks

If one can infer the usage of unix sockets from other parameters passed anyway, then there may be no point having it in the first place. For the VNC URI, we could have gone with vnc+ssh://, or vnc+ssl:// (for stunnel), but there were ssh-related and stunnel parameters we could infer from so it

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

On Thu, Feb 15, 2018 at 04:04:57PM +0100, Christophe de Dinechin wrote: > > This style guide only indicates what we aim to achieve. It does not > necessarily reflect the current state of the code. > > What about adding: > > Consistency matters. It may be preferable to ignore a

[Spice-devel] [PATCH v4 07/12] Remove confusing example

From: Christophe de Dinechin The sentence explaining that example makes no real sense, the mentioned style is not used by the current code, and the coding style suggestion is horrendous. Not to mention that it flies in the face of all automatic indentation tools I know of.

[Spice-devel] [PATCH v4 06/12] Rephrase section on short functions for readability

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- docs/spice_style.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/spice_style.txt b/docs/spice_style.txt index 996c5cd9..9e6536e4 100644 ---

[Spice-devel] [PATCH v4 11/12] Point out that the coding style is for all SPICE components

From: Christophe de Dinechin Historically, the coding style was created in the server component, and that's where it's built from. However, it applies also to other components, and is presently being reworked in large part because of the introduction of a large amount of C++

[Spice-devel] [PATCH v4 00/12] Updates to the style guide

From: Christophe de Dinechin Changes in v4: - Integrate the many comments made on list Details in the respective commit logs Changes in v3: - Integrated clarification regarding headers - Added external references as a starting point - Changed [source,h] annotations

[Spice-devel] [PATCH v4 12/12] Whitespace adjustment guideline

From: Christophe de Dinechin Change since v3: - This particular aspect is controversial, so it was put in a separate patch. Signed-off-by: Christophe de Dinechin --- docs/spice_style.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git

[Spice-devel] [PATCH v4 08/12] Add guidelines about warnings

From: Christophe de Dinechin The objective of these guidelines is that: - We avoid introducing new warnings - We know how to fix old ones Changes since v3: - Put 'controversial' whitespace proposal in separate patch Signed-off-by: Christophe de Dinechin

[Spice-devel] [PATCH v4 09/12] Fix the style guide for headers

From: Christophe de Dinechin As written, the headers style guide looks quite wrong. In particular, it places headers in an order that makes it hard to detect hidden dependencies in SPICE headers. These rules can be enforced by the .clang-format proposed in earlier patch,

[Spice-devel] [PATCH v4 03/12] Specify file extensions for C++ and Obj-C

From: Christophe de Dinechin Note that we don't have .m files in the repository yet, but some files actually require building as Objective-C on some platformsdue to system header dependencies (e.g. . This is currently worked around by passing the CFLAGS=-ObjC command-line

[Spice-devel] [PATCH v4 05/12] Rephrase section about constants

From: Christophe de Dinechin The indent of the rephrasing is that: - If you have a single constant, use const, e.g. (visible in debugger) const unsigned max_stuff = 42; - If you have multiple constants, prefer enums over #define, as already suggested later in the

[Spice-devel] [PATCH v4 04/12] Rephrase assertion section

From: Christophe de Dinechin Changes since v3: - Integrate comments about performance impact and solution - Integrate comments about impact of a failed assertion - Add prohibition against side effects Signed-off-by: Christophe de Dinechin ---

[Spice-devel] [PATCH v4 01/12] Add .clang-format with defaults matching what's specified in the style guide

From: Christophe de Dinechin There are two use cases for this .clang-format: 1. Local reformatting of source code, e.g. using Emacs clang-format.el package. This is basically a wawy to accelerate routine reformatting actions during heavy editing or refactoring. 2.

[Spice-devel] [PATCH v4 10/12] Integrate suggestions from Christophe Fergeau

From: Christophe de Dinechin This attempts to find a wording that takes into account the existing practice, notably in the server. Signed-off-by: Christophe de Dinechin --- docs/spice_style.txt | 16 +--- 1 file changed, 13 insertions(+),

[Spice-devel] [PATCH v4 02/12] Added External References section

From: Christophe de Dinechin Changes since v3: - Clarify that the guidelines are an objective, not the state of the code - Clarify that we try to have a unique style between C and C++ - Clarify that existing practice and code consistency matter Signed-off-by: Christophe de

Re: [Spice-devel] [RFC PATCH 1/1] separate and encapsulate the agent business code

On Thu, 2018-02-15 at 13:03 +0100, Christophe de Dinechin wrote: > > On 15 Feb 2018, at 11:51, Lukáš Hrázký wrote: > > > > On Wed, 2018-02-14 at 22:13 +0100, Christophe de Dinechin wrote: > > > I cut quite a bit for clarity. What I cut I agree with ;-) > > > > > > > > > >

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

> On 15 Feb 2018, at 15:55, Christophe Fergeau wrote: > > On Thu, Feb 15, 2018 at 03:25:23PM +0100, Christophe de Dinechin wrote: >> >> >>> On 15 Feb 2018, at 13:41, Christophe Fergeau wrote: >>> >>> On Thu, Feb 15, 2018 at 11:55:44AM +0100,

Re: [Spice-devel] [PATCH v3 11/11] Rewrite the style guide for headers

On Thu, Feb 15, 2018 at 03:25:08PM +0100, Christophe de Dinechin wrote: > > And then I made concrete suggestions > > as how I would move forward with this patch… > > Sorry, I saw your push back, but I did not see the concrete suggestion for > moving forward… Would you please be kind enough to

Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/4] Remove clang warning on missing 'override'

> On 15 Feb 2018, at 15:25, Frediano Ziglio wrote: > >>> On 15 Feb 2018, at 11:00, Frediano Ziglio wrote: >>> >>> From: Christophe de Dinechin >>> >>> In file included from mjpeg-fallback.cpp:8: >>> ./mjpeg-fallback.hpp:28:25:

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

On Thu, Feb 15, 2018 at 03:25:23PM +0100, Christophe de Dinechin wrote: > > > > On 15 Feb 2018, at 13:41, Christophe Fergeau wrote: > > > > On Thu, Feb 15, 2018 at 11:55:44AM +0100, Christophe de Dinechin wrote: > >> Now, Christophe’s arguments are that > >> > >> 1) we

Re: [Spice-devel] [PATCH v3 01/11] Add .clang-format with defaults matching what's specified in the style guide

> > > On 15 Feb 2018, at 11:59, Frediano Ziglio wrote: > > > >>> > >>> On 15 Feb 2018, at 10:07, Christophe Fergeau wrote: > >>> > >>> On Wed, Feb 14, 2018 at 10:29:25PM +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at

Re: [Spice-devel] [PATCH 08/14] Use RAII to cleanup stream in case of exception or return

> > > On 15 Feb 2018, at 11:04, Frediano Ziglio wrote: > > > >> > >> From: Christophe de Dinechin > >> > >> This lets us get rid of C-style 'goto done' in do_capture. > >> > >> Signed-off-by: Christophe de Dinechin > > > > I

[Spice-devel] #include "canvas_base.c" ?

While looking at some warnings, I came across this in sw_canvas.c: #include “canvas_base.c" So we include a .c in another one. Apparently, this was inherited from some Cairo canvas. Is that something we care to keep as is? canvas_base.c by itself does compile, but with warnings about unused

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

> On 15 Feb 2018, at 13:41, Christophe Fergeau wrote: > > On Thu, Feb 15, 2018 at 11:55:44AM +0100, Christophe de Dinechin wrote: >> Now, Christophe’s arguments are that >> >> 1) we should not write guidelines that are inconsistent with existing code. >> 2) this is in the

Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/4] Remove clang warning on missing 'override'

> > On 15 Feb 2018, at 11:00, Frediano Ziglio wrote: > > > > From: Christophe de Dinechin > > > > In file included from mjpeg-fallback.cpp:8: > > ./mjpeg-fallback.hpp:28:25: warning: 'VideoCodecType' overrides a member > > function but is not marked

Re: [Spice-devel] [PATCH v3 11/11] Rewrite the style guide for headers

> On 15 Feb 2018, at 13:47, Christophe Fergeau wrote: > > On Thu, Feb 15, 2018 at 01:14:43PM +0100, Christophe de Dinechin wrote: >> >> >>> On 15 Feb 2018, at 10:19, Christophe Fergeau wrote: >>> >>> On Wed, Feb 14, 2018 at 11:24:50PM +0100,

Re: [Spice-devel] [PATCH] Remove trailing whitespace

On Thu, 2018-02-15 at 13:29 +0100, Christophe de Dinechin wrote: > From: Christophe de Dinechin > > Signed-off-by: Christophe de Dinechin > --- > src/mjpeg-fallback.cpp | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git

Re: [Spice-devel] [PATCH] Remove trailing whitespace

On 02/15/2018 02:29 PM, Christophe de Dinechin wrote: From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/mjpeg-fallback.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mjpeg-fallback.cpp

Re: [Spice-devel] [PATCH v3 11/11] Rewrite the style guide for headers

On Thu, Feb 15, 2018 at 01:14:43PM +0100, Christophe de Dinechin wrote: > > > > On 15 Feb 2018, at 10:19, Christophe Fergeau wrote: > > > > On Wed, Feb 14, 2018 at 11:24:50PM +0100, Christophe de Dinechin wrote: > >> > >> > >>> On 14 Feb 2018, at 14:35, Christophe

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

On Thu, Feb 15, 2018 at 11:55:44AM +0100, Christophe de Dinechin wrote: > Now, Christophe’s arguments are that > > 1) we should not write guidelines that are inconsistent with existing code. > 2) this is in the server codebase, so we should server rules > > Problem is with 2, really. > > We

Re: [Spice-devel] [PATCH spice-streaming-agent v2 4/4] log_binary is really a boolean

> On 15 Feb 2018, at 11:00, Frediano Ziglio wrote: > > From: Christophe de Dinechin > > Signed-off-by: Christophe de Dinechin > --- > Change since v1: > - do not clash with possible short 'b' option. > --- >

Re: [Spice-devel] [PATCH spice-streaming-agent v2 2/4] Remove clang warning on missing 'override'

> On 15 Feb 2018, at 11:00, Frediano Ziglio wrote: > > From: Christophe de Dinechin > > In file included from mjpeg-fallback.cpp:8: > ./mjpeg-fallback.hpp:28:25: warning: 'VideoCodecType' overrides a member > function but is not marked 'override'

[Spice-devel] [PATCH] Remove trailing whitespace

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- src/mjpeg-fallback.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp index 74682f3..8238233 100644 ---

Re: [Spice-devel] [PATCH v3 11/11] Rewrite the style guide for headers

> On 15 Feb 2018, at 10:19, Christophe Fergeau wrote: > > On Wed, Feb 14, 2018 at 11:24:50PM +0100, Christophe de Dinechin wrote: >> >> >>> On 14 Feb 2018, at 14:35, Christophe Fergeau wrote: >>> >>> This one sounds more like an RFC to me >> >>

Re: [Spice-devel] [RFC PATCH 1/1] separate and encapsulate the agent business code

> On 15 Feb 2018, at 11:51, Lukáš Hrázký wrote: > > On Wed, 2018-02-14 at 22:13 +0100, Christophe de Dinechin wrote: >> I cut quite a bit for clarity. What I cut I agree with ;-) >> >> >>> On 14 Feb 2018, at 14:45, Lukáš Hrázký wrote: >>> >>> On Tue,

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

> > > On 15 Feb 2018, at 11:43, Frediano Ziglio wrote: > > > >> > >> On Thu, Feb 15, 2018 at 10:56:49AM +0100, Lukáš Hrázký wrote: > >>> On Wed, 2018-02-14 at 22:43 +0100, Christophe de Dinechin wrote: > > On 14 Feb 2018, at 17:29, Christophe Fergeau

Re: [Spice-devel] [PATCH v3 11/11] Rewrite the style guide for headers

On Wed, 2018-02-14 at 23:24 +0100, Christophe de Dinechin wrote: > > On 14 Feb 2018, at 14:35, Christophe Fergeau wrote: > > > > This one sounds more like an RFC to me > > Well, this is really a bug fix in the documentation more than a RFC. > > > , as from a quick look in

Re: [Spice-devel] [PATCH v3 01/11] Add .clang-format with defaults matching what's specified in the style guide

> On 15 Feb 2018, at 11:59, Frediano Ziglio wrote: > >>> >>> On 15 Feb 2018, at 10:07, Christophe Fergeau wrote: >>> >>> On Wed, Feb 14, 2018 at 10:29:25PM +0100, Christophe de Dinechin wrote: > On 14 Feb 2018, at 17:34, Christophe

Re: [Spice-devel] [PATCH 08/14] Use RAII to cleanup stream in case of exception or return

> On 15 Feb 2018, at 11:04, Frediano Ziglio wrote: > >> >> From: Christophe de Dinechin >> >> This lets us get rid of C-style 'goto done' in do_capture. >> >> Signed-off-by: Christophe de Dinechin > > I honestly prefer the

Re: [Spice-devel] [PATCH v3 01/11] Add .clang-format with defaults matching what's specified in the style guide

> > > On 15 Feb 2018, at 10:07, Christophe Fergeau wrote: > > > > On Wed, Feb 14, 2018 at 10:29:25PM +0100, Christophe de Dinechin wrote: > >> > >> > >>> On 14 Feb 2018, at 17:34, Christophe Fergeau wrote: > >>> > >>> On Wed, Feb 14, 2018 at

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

> On 15 Feb 2018, at 11:43, Frediano Ziglio wrote: > >> >> On Thu, Feb 15, 2018 at 10:56:49AM +0100, Lukáš Hrázký wrote: >>> On Wed, 2018-02-14 at 22:43 +0100, Christophe de Dinechin wrote: > On 14 Feb 2018, at 17:29, Christophe Fergeau >

Re: [Spice-devel] [RFC PATCH 1/1] separate and encapsulate the agent business code

On Wed, 2018-02-14 at 22:13 +0100, Christophe de Dinechin wrote: > I cut quite a bit for clarity. What I cut I agree with ;-) > > > > On 14 Feb 2018, at 14:45, Lukáš Hrázký wrote: > > > > On Tue, 2018-02-13 at 17:10 +0100, Christophe de Dinechin wrote: > > > Hi Lukas, > > >

Re: [Spice-devel] [PATCH v3 01/11] Add .clang-format with defaults matching what's specified in the style guide

> On 15 Feb 2018, at 10:07, Christophe Fergeau wrote: > > On Wed, Feb 14, 2018 at 10:29:25PM +0100, Christophe de Dinechin wrote: >> >> >>> On 14 Feb 2018, at 17:34, Christophe Fergeau wrote: >>> >>> On Wed, Feb 14, 2018 at 10:45:56AM -0500,

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

> > On Thu, Feb 15, 2018 at 10:56:49AM +0100, Lukáš Hrázký wrote: > > On Wed, 2018-02-14 at 22:43 +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at 17:29, Christophe Fergeau > > > > wrote: > > > > > > > > This changes the suggested style to what is currently

Re: [Spice-devel] [PATCH spice-streaming-agent v3 0/3] first unit test and options parsing improvements

> > This series introduces a C++ unit test framework called Catch to the > codebase, adds a simple unit test for the options parsing for the mjpeg > plugin and improves on the option parsing code. > > Since we more or less agreed we can solve the Catch package in RHEL one > way or another, I

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

On Thu, Feb 15, 2018 at 10:56:49AM +0100, Lukáš Hrázký wrote: > On Wed, 2018-02-14 at 22:43 +0100, Christophe de Dinechin wrote: > > > On 14 Feb 2018, at 17:29, Christophe Fergeau wrote: > > > > > > This changes the suggested style to what is currently used in > > >

Re: [Spice-devel] [PATCH 08/14] Use RAII to cleanup stream in case of exception or return

> > From: Christophe de Dinechin > > This lets us get rid of C-style 'goto done' in do_capture. > > Signed-off-by: Christophe de Dinechin I honestly prefer the "defer" style way. Beside that you correctly pointed out that there's a race condition on

[Spice-devel] [PATCH spice-streaming-agent v2 0/4] Minor updates for coding style changes

Updated some proposed patches for the "Minor improvements and coding style changes to the streaming agent" series. See relative patches. Rebase also on master. Christophe de Dinechin (4): Rename 'quit' to 'quit_requested' for consistency with 'streaming_requested' Remove clang warning on

[Spice-devel] [PATCH spice-streaming-agent v2 3/4] Add PRIu64 format for uint64_t

From: Christophe de Dinechin Otherwise, clang complains: spice-streaming-agent.cpp:414:66: warning: format specifies type 'unsigned long' but the argument has type 'uint64_t' (aka 'unsigned long long') [-Wformat] fprintf(f_log, "%lu: Frame of %zu

[Spice-devel] [PATCH spice-streaming-agent v2 1/4] Rename 'quit' to 'quit_requested' for consistency with 'streaming_requested'

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin Acked-by: Frediano Ziglio --- Change since v1: - fixed bracket of changed line. --- src/spice-streaming-agent.cpp | 15 --- 1 file changed, 8

[Spice-devel] [PATCH spice-streaming-agent v2 2/4] Remove clang warning on missing 'override'

From: Christophe de Dinechin In file included from mjpeg-fallback.cpp:8: ./mjpeg-fallback.hpp:28:25: warning: 'VideoCodecType' overrides a member function but is not marked 'override' [-Winconsistent-missing-override] SpiceVideoCodecType VideoCodecType() const;

[Spice-devel] [PATCH spice-streaming-agent v2 4/4] log_binary is really a boolean

From: Christophe de Dinechin Signed-off-by: Christophe de Dinechin --- Change since v1: - do not clash with possible short 'b' option. --- src/spice-streaming-agent.cpp | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

On Wed, 2018-02-14 at 22:43 +0100, Christophe de Dinechin wrote: > > On 14 Feb 2018, at 17:29, Christophe Fergeau wrote: > > > > This changes the suggested style to what is currently used in > > spice-server codebase. This also removes a few sentences which > > are not

Re: [Spice-devel] [PATCH 04/14] Add PRIu64 format for uint64_t

> > > > From: Christophe de Dinechin > > > > Otherwise, clang complains: > > > > spice-streaming-agent.cpp:414:66: warning: format specifies type 'unsigned > > long' but the argument has type 'uint64_t' (aka 'unsigned long long') > > [-Wformat] > >

Re: [Spice-devel] [spice-server] style: Slight tweak to the header guard section

On Wed, Feb 14, 2018 at 10:43:26PM +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at 17:29, Christophe Fergeau wrote: > > > > This changes the suggested style to what is currently used in > > spice-server codebase. This also removes a few sentences which > >

Re: [Spice-devel] [PATCH v3 11/11] Rewrite the style guide for headers

On Wed, Feb 14, 2018 at 11:24:50PM +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at 14:35, Christophe Fergeau wrote: > > > > This one sounds more like an RFC to me > > Well, this is really a bug fix in the documentation more than a RFC. > > > , as from a

Re: [Spice-devel] [PATCH v3 01/11] Add .clang-format with defaults matching what's specified in the style guide

On Wed, Feb 14, 2018 at 10:29:25PM +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at 17:34, Christophe Fergeau wrote: > > > > On Wed, Feb 14, 2018 at 10:45:56AM -0500, Frediano Ziglio wrote: > >>> > >>> Shouldn't this go with a Makefile rule? A few lines in

Re: [Spice-devel] [PATCH v3 10/11] Add guidelines about warnings and whitespaces

On Wed, Feb 14, 2018 at 10:53:04PM +0100, Christophe de Dinechin wrote: > > > > On 14 Feb 2018, at 14:37, Christophe Fergeau wrote: > > > > On Thu, Feb 08, 2018 at 12:25:30PM +0100, Christophe de Dinechin wrote: > >> From: Christophe de Dinechin > >>

[Spice-devel] [spice-gtk v1 2/2] channel-display: use libva to set preferred video codecs

From: Victor Toso By using the detection of which video codecs (and profiles!) the hardware supports with VA-API capable driver in previous commit. Signed-off-by: Victor Toso --- src/channel-display.c | 33 - 1 file

[Spice-devel] [spice-gtk v1 0/2] Using libva to detect hardware capabilities

From: Victor Toso For VA-API cable drivers at least, this should set the video codecs order based on hw capabilities. We can use this to improve/set limitation on possible streams too, like don't send 4K stream if we _know_ that HW can support only Full HD... but we lack

[Spice-devel] [spice-gtk v1 1/2] Use libva to check video hardware accel capabilities

From: Victor Toso Libva is an implementation for VA-API. This can be used to automatically send to the server the preferred video codecs as the client would prefer streams with video codecs that can be decoded with gpu support. We can also use the profiles to detect and