>
> This patch gives more priority to the requested video codecs when
> selecting the FrameCapture plugin, instead of its hard-coded rank.
>
> 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@xxxxxxxxxx>
> ---
> 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;
I think that previous code was entirely in favour of agent preference
but the new one looks too much in favour of the client.
Also adding preference to a message not designed to do so is not
100% correct. It's not a big hack but was not designed to do so.
For instance is impossible for the client to specify that 2 codecs
have the same preference or how much one should be favourite instead
of the other.
I would generate a rank that take in account both agent and client
preference and use the sorted_plugins as was used before instead
of taking an already sorted array and ignoring most of the sorting
and ranking.
Maybe normalize rank for agent and client to a 0-255 range and
compute:
total_rank = agent_rank * 16 + client_rank;
?? this will favour the agent but still give the possibility
for the client to override a bit.
The client_rank should come from a similar start/stop message with
added rank.
> 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 {};
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel