Re: [Spice-devel] [PATCH v2 16/24] Remove client_codecs global variable, moved inside the 'Stream' class

[Frediano Ziglio]

> 
> From: Christophe de Dinechin 
> 
> This is not a really nice abstraction at this point, but still a step in the
> right way
> 

This comment is weird.

> Signed-off-by: Christophe de Dinechin 
> ---
>  src/spice-streaming-agent.cpp | 16 ++--
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index 2c38233..d38590f 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -61,8 +61,11 @@ static uint64_t get_time(void)
>  
>  class Stream
>  {
> +typedef std::set codecs_t;
> +
>  public:
>  Stream(const char *name)
> +: codecs()
>  {
>  streamfd = open(name, O_RDWR);
>  if (streamfd < 0)

Here initialization is useless.

> @@ -73,6 +76,8 @@ public:
>  close(streamfd);
>  }
>  
> +const codecs_t _codecs() { return codecs; }
> +

I would say is a const method

>  int have_something_to_read(int timeout);
>  void handle_stream_start_stop(uint32_t len);
>  void handle_stream_capabilities(uint32_t len);
> @@ -80,7 +85,6 @@ public:
>  void read_command_from_device(void);
>  int read_command(bool blocking);
>  
> -
>  template 
>  bool send(PayloadArgs... payload)
>  {

This should be squashed in the patch introducing it.

> @@ -99,6 +103,7 @@ public:
>  private:
>  int streamfd = -1;
>  std::mutex mutex;
> +codecs_t codecs;
>  };
>  
>  
> @@ -318,7 +323,6 @@ void X11CursorThread::cursor_changes()
>  
>  static bool streaming_requested = false;
>  static bool quit_requested = false;
> -static std::set client_codecs;
>  
>  int Stream::have_something_to_read(int timeout)
>  {
> @@ -352,9 +356,9 @@ void Stream::handle_stream_start_stop(uint32_t len)
>  streaming_requested = (msg[0] != 0); /* num_codecs */
>  syslog(LOG_INFO, "GOT START_STOP message -- request to %s streaming\n",
> streaming_requested ? "START" : "STOP");
> -client_codecs.clear();
> +codecs.clear();
>  for (int i = 1; i <= msg[0]; ++i) {
> -client_codecs.insert((SpiceVideoCodecType) msg[i]);
> +codecs.insert((SpiceVideoCodecType) msg[i]);
>  }
>  }
>  
> @@ -378,7 +382,7 @@ void Stream::handle_stream_capabilities(uint32_t len)
>  STREAM_TYPE_CAPABILITIES,
>  0
>  };
> -if (write_all(, sizeof(hdr)) != sizeof(hdr)) {
> +if (write_all("Request Capabilities", , sizeof(hdr)) != sizeof(hdr))
> {
>  throw std::runtime_error("error writing capabilities");
>  }
>  }
> @@ -503,7 +507,7 @@ do_capture(Stream , const char *streamport,
> FrameLog _log)
>  syslog(LOG_INFO, "streaming starts now\n");
>  uint64_t time_last = 0;
>  
> -std::unique_ptr
> capture(agent.GetBestFrameCapture(client_codecs));
> +std::unique_ptr
> capture(agent.GetBestFrameCapture(stream.client_codecs()));
>  if (!capture)
>  throw std::runtime_error("cannot find a suitable capture
>  system");
>  

Frediano
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Spice-devel] [PATCH v2 16/24] Remove client_codecs global variable, moved inside the 'Stream' class

[Christophe de Dinechin]

From: Christophe de Dinechin 

This is not a really nice abstraction at this point, but still a step in the 
right way

Signed-off-by: Christophe de Dinechin 
---
 src/spice-streaming-agent.cpp | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
index 2c38233..d38590f 100644
--- a/src/spice-streaming-agent.cpp
+++ b/src/spice-streaming-agent.cpp
@@ -61,8 +61,11 @@ static uint64_t get_time(void)
 
 class Stream
 {
+typedef std::set codecs_t;
+
 public:
 Stream(const char *name)
+: codecs()
 {
 streamfd = open(name, O_RDWR);
 if (streamfd < 0)
@@ -73,6 +76,8 @@ public:
 close(streamfd);
 }
 
+const codecs_t _codecs() { return codecs; }
+
 int have_something_to_read(int timeout);
 void handle_stream_start_stop(uint32_t len);
 void handle_stream_capabilities(uint32_t len);
@@ -80,7 +85,6 @@ public:
 void read_command_from_device(void);
 int read_command(bool blocking);
 
-
 template 
 bool send(PayloadArgs... payload)
 {
@@ -99,6 +103,7 @@ public:
 private:
 int streamfd = -1;
 std::mutex mutex;
+codecs_t codecs;
 };
 
 
@@ -318,7 +323,6 @@ void X11CursorThread::cursor_changes()
 
 static bool streaming_requested = false;
 static bool quit_requested = false;
-static std::set client_codecs;
 
 int Stream::have_something_to_read(int timeout)
 {
@@ -352,9 +356,9 @@ void Stream::handle_stream_start_stop(uint32_t len)
 streaming_requested = (msg[0] != 0); /* num_codecs */
 syslog(LOG_INFO, "GOT START_STOP message -- request to %s streaming\n",
streaming_requested ? "START" : "STOP");
-client_codecs.clear();
+codecs.clear();
 for (int i = 1; i <= msg[0]; ++i) {
-client_codecs.insert((SpiceVideoCodecType) msg[i]);
+codecs.insert((SpiceVideoCodecType) msg[i]);
 }
 }
 
@@ -378,7 +382,7 @@ void Stream::handle_stream_capabilities(uint32_t len)
 STREAM_TYPE_CAPABILITIES,
 0
 };
-if (write_all(, sizeof(hdr)) != sizeof(hdr)) {
+if (write_all("Request Capabilities", , sizeof(hdr)) != sizeof(hdr)) {
 throw std::runtime_error("error writing capabilities");
 }
 }
@@ -503,7 +507,7 @@ do_capture(Stream , const char *streamport, FrameLog 
_log)
 syslog(LOG_INFO, "streaming starts now\n");
 uint64_t time_last = 0;
 
-std::unique_ptr 
capture(agent.GetBestFrameCapture(client_codecs));
+std::unique_ptr 
capture(agent.GetBestFrameCapture(stream.client_codecs()));
 if (!capture)
 throw std::runtime_error("cannot find a suitable capture system");
 
-- 
2.13.5 (Apple Git-94)

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel