[Spice-devel] [RFC spice-streaming-agent 4/4] concrete-agent: prioritize requested codec for plugin selection

[Snir Sheriber]

On 8/6/19 6:34 PM, Kevin Pouget wrote:
> This patch gives more priority to the requested video codecs when
> selecting the FrameCapture plugin, instead of its hard-coded rank.


We may want to mention this priority also in the header file.

Snir.

> The client_codecs storage structure is changed from 'set' to 'vector',
> as the codec order is not preserved by the set structure..
>
> Signed-off-by: Kevin Pouget <kpouget at redhat.com>
> ---
>   src/concrete-agent.cpp        | 38 +++++++++++++++++------------------
>   src/concrete-agent.hpp        |  2 +-
>   src/spice-streaming-agent.cpp |  2 +-
>   src/stream-port.cpp           |  2 +-
>   src/stream-port.hpp           |  4 ++--
>   5 files changed, 24 insertions(+), 24 deletions(-)
>
> diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp
> index 336bd09..683fa26 100644
> --- a/src/concrete-agent.cpp
> +++ b/src/concrete-agent.cpp
> @@ -111,7 +111,7 @@ void ConcreteAgent::LoadPlugin(const std::string &plugin_filename)
>       }
>   }
>   
> -FrameCapture *ConcreteAgent::GetBestFrameCapture(const std::set<SpiceVideoCodecType>& codecs)
> +FrameCapture *ConcreteAgent::GetBestFrameCapture(const std::vector<SpiceVideoCodecType>& codecs)
>   {
>       std::vector<std::pair<unsigned, std::shared_ptr<Plugin>>> sorted_plugins;
>   
> @@ -121,24 +121,24 @@ FrameCapture *ConcreteAgent::GetBestFrameCapture(const std::set<SpiceVideoCodecT
>       }
>       sort(sorted_plugins.rbegin(), sorted_plugins.rend());
>   
> -    // return first not null
> -    for (const auto& plugin: sorted_plugins) {
> -        if (plugin.first == DontUse) {
> -            break;
> -        }
> -        // check client supports the codec
> -        if (codecs.find(plugin.second->VideoCodecType()) == codecs.end())
> -            continue;
> -
> -        FrameCapture *capture;
> -        try {
> -            capture = plugin.second->CreateCapture();
> -        } catch (const std::exception &err) {
> -            syslog(LOG_ERR, "Error creating capture engine: %s", err.what());
> -            continue;
> -        }
> -        if (capture) {
> -            return capture;
> +    for (const auto& codec_type: codecs) {
> +        // find the plugin with the highest rank for this codec type
> +        for (const auto& plugin: sorted_plugins) {
> +            if (plugin.first == DontUse) {
> +                continue;
> +            }
> +
> +            // check client supports the codec
> +            if (plugin.second->VideoCodecType() != codec_type) {
> +                continue;
> +            }
> +
> +            try {
> +                return plugin.second->CreateCapture();
> +            } catch (const std::exception &err) {
> +                syslog(LOG_ERR, "Error creating capture engine: %s", err.what());
> +                continue;
> +            }
>           }
>       }
>       return nullptr;
> diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp
> index 6d1be35..0399bab 100644
> --- a/src/concrete-agent.hpp
> +++ b/src/concrete-agent.hpp
> @@ -33,7 +33,7 @@ public:
>       void Register(const std::shared_ptr<Plugin>& plugin) override;
>       const ConfigureOption* Options() const override;
>       void LoadPlugins(const std::string &directory);
> -    FrameCapture *GetBestFrameCapture(const std::set<SpiceVideoCodecType>& codecs);
> +    FrameCapture *GetBestFrameCapture(const std::vector<SpiceVideoCodecType>& codecs);
>       __attribute__ ((format (printf, 2, 3)))
>       void LogStat(const char* format, ...) override;
>   private:
> diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming-agent.cpp
> index d274b5f..15c2732 100644
> --- a/src/spice-streaming-agent.cpp
> +++ b/src/spice-streaming-agent.cpp
> @@ -128,7 +128,7 @@ private:
>   static bool capture_in_progress = false;
>   static bool reset_requested = false;
>   static bool quit_requested = false;
> -static std::set<SpiceVideoCodecType> client_codecs;
> +static std::vector<SpiceVideoCodecType> client_codecs;
>   
>   static bool have_something_to_read(StreamPort &stream_port, bool blocking)
>   {
> diff --git a/src/stream-port.cpp b/src/stream-port.cpp
> index 2670120..0d08d50 100644
> --- a/src/stream-port.cpp
> +++ b/src/stream-port.cpp
> @@ -43,7 +43,7 @@ StartStopMessage InboundMessage::get_payload<StartStopMessage>()
>       }
>   
>       for (size_t i = 1; i <= data[0]; ++i) {
> -        msg.client_codecs.insert((SpiceVideoCodecType) data[i]);
> +        msg.client_codecs.push_back((SpiceVideoCodecType) data[i]);
>       }
>   
>       return msg;
> diff --git a/src/stream-port.hpp b/src/stream-port.hpp
> index 08473f7..b2f7520 100644
> --- a/src/stream-port.hpp
> +++ b/src/stream-port.hpp
> @@ -16,7 +16,7 @@
>   #include <string>
>   #include <memory>
>   #include <mutex>
> -#include <set>
> +#include <vector>
>   
>   
>   namespace spice {
> @@ -45,7 +45,7 @@ public:
>   struct StartStopMessage
>   {
>       bool start_streaming = false;
> -    std::set<SpiceVideoCodecType> client_codecs;
> +    std::vector<SpiceVideoCodecType> client_codecs;
>   };
>   
>   struct InCapabilitiesMessage {};