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

[Christophe de Dinechin]

> On 20 Feb 2018, at 16:51, Lukáš Hrázký <lhrazky at redhat.com> wrote:
> 
> On Fri, 2018-02-16 at 17:15 +0100, Christophe de Dinechin wrote:
>> From: Christophe de Dinechin <dinechin at redhat.com>
>> 
>> This is not a really nice abstraction at this point, but still a step in the right way
>> 
>> Signed-off-by: Christophe de Dinechin <dinechin at redhat.com>
>> ---
>> src/spice-streaming-agent.cpp | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>> 
>> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
>> index b0c09d8..19f3c07 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<SpiceVideoCodecType> codecs_t;
>> +
>> public:
>>     Stream(const char *name)
>> +        : codecs()
>>     {
>>         streamfd = open(name, O_RDWR);
>>         if (streamfd < 0)
>> @@ -73,11 +76,12 @@ public:
>>         close(streamfd);
>>     }
>> 
>> +    const codecs_t &client_codecs() { return codecs; }
>> +
>>     int have_something_to_read(int timeout);
>>     int read_command_from_device(void);
>>     int read_command(bool blocking);
>> 
>> -
>>     template <typename Message, typename ...PayloadArgs>
>>     bool send(PayloadArgs... payload)
>>     {
>> @@ -96,6 +100,7 @@ public:
>> private:
>>     int streamfd = -1;
>>     std::mutex mutex;
>> +    codecs_t codecs;
>> };
>> 
>> 
>> @@ -312,7 +317,6 @@ void X11CursorThread::cursor_changes()
>> 
>> static bool streaming_requested = false;
>> static bool quit_requested = false;
>> -static std::set<SpiceVideoCodecType> client_codecs;
>> 
>> int Stream::have_something_to_read(int timeout)
>> {
>> @@ -369,9 +373,9 @@ int Stream::read_command_from_device()
>>     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]);
>>     return 1;
>> }
>> 
>> @@ -461,7 +465,7 @@ do_capture(Stream &stream, const char *streamport, FrameLog &frame_log)
>>         syslog(LOG_INFO, "streaming starts now\n");
>>         uint64_t time_last = 0;
>> 
>> -        std::unique_ptr<FrameCapture> capture(agent.GetBestFrameCapture(client_codecs));
>> +        std::unique_ptr<FrameCapture> capture(agent.GetBestFrameCapture(stream.client_codecs()));
>>         if (!capture)
>>             throw std::runtime_error("cannot find a suitable capture system");
> 
> How about we leave this patch out and do it properly straight away? I can do it…

I don’t mind, but maybe it’s still enough progress that we keep it, and then we have all your refactoring (including input messages) in one place?

(Here, I’m concerned about tracing the evolution of the code. The step I did seems obvious from the existing code. Maybe yours is too? Not having looked at it yet, I don’t know)

I don’t mind either way.

> 
> Lukas
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel