> > On Tue, 2017-11-21 at 09:49 +0000, Frediano Ziglio wrote: > > This allows for instance old clients to work correctly. > > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > > --- > > src/concrete-agent.cpp | 5 ++++- > > src/concrete-agent.hpp | 3 ++- > > src/spice-streaming-agent.cpp | 6 +++++- > > 3 files changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp > > index 192054a..404ee74 100644 > > --- a/src/concrete-agent.cpp > > +++ b/src/concrete-agent.cpp > > @@ -98,7 +98,7 @@ void ConcreteAgent::LoadPlugin(const char > > *plugin_filename) > > } > > } > > > > -FrameCapture *ConcreteAgent::GetBestFrameCapture() > > +FrameCapture *ConcreteAgent::GetBestFrameCapture(const > > std::set<SpiceVideoCodecType>& codecs) > > > I notice that there's a bit of inconsistency about specifying the std:: > namespace or just using the bare type. There is a "using namespace > std;" statement in this file, and most standard library types are used > without specifying the namespace. But there there is at least one place > where we specify the namespace, and you've added another in this patch. > Usually I tend to use in the header to avoid having a global unwanted "using namespace" statement and avoid in the source (at least for std). We can define to always use the explicit namespace for "std". > (as a matter of personal preference, I generally dislike 'using > namespace' statements, and I prefer to specify the std:: namespace > explicitly) > > > > > > > { > > vector<pair<unsigned, shared_ptr<Plugin>>> sorted_plugins; > > > > @@ -113,6 +113,9 @@ FrameCapture > > *ConcreteAgent::GetBestFrameCapture() > > if (plugin.first == DontUse) { > > break; > > Unrelated to this patch, but shouldn't the statement above be > 'continue' rather than 'break'? > They are sorted based on rank so all others are DontUse too. > > > } > > + // check client supports the codec > > + if (codecs.find(plugin.second->VideoCodecType()) == > > codecs.end()) > > + continue; > > FrameCapture *capture = plugin.second->CreateCapture(); > > if (capture) { > > return capture; > > diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp > > index 828368b..6c300fa 100644 > > --- a/src/concrete-agent.hpp > > +++ b/src/concrete-agent.hpp > > @@ -7,6 +7,7 @@ > > #define SPICE_STREAMING_AGENT_CONCRETE_AGENT_HPP > > > > #include <vector> > > +#include <set> > > #include <memory> > > #include <spice-streaming-agent/plugin.hpp> > > > > @@ -33,7 +34,7 @@ public: > > void LoadPlugins(const char *directory); > > // pointer must remain valid > > void AddOption(const char *name, const char *value); > > - FrameCapture *GetBestFrameCapture(); > > + FrameCapture *GetBestFrameCapture(const > > std::set<SpiceVideoCodecType>& codecs); > bool PluginVersionIsCompatible(unsigned pluginVersion) const > > override; > > private: > > void LoadPlugin(const char *plugin_filename); > > diff --git a/src/spice-streaming-agent.cpp b/src/spice-streaming- > > agent.cpp > > index 20f2925..5c0bbb9 100644 > > --- a/src/spice-streaming-agent.cpp > > +++ b/src/spice-streaming-agent.cpp > > @@ -52,6 +52,7 @@ struct SpiceStreamDataMessage > > }; > > > > static int streaming_requested; > > +static std::set<SpiceVideoCodecType> client_codecs; > > static bool quit; > > static int streamfd = -1; > > static bool stdin_ok; > > @@ -143,6 +144,9 @@ static int read_command_from_device(void) > > streaming_requested = msg[0]; /* num_codecs */ > > syslog(LOG_INFO, "GOT START_STOP message -- request to %s > > streaming\n", > > streaming_requested ? "START" : "STOP"); > > + client_codecs.clear(); > > + for (int i = 1; i <= msg[0]; ++i) > > + client_codecs.insert((SpiceVideoCodecType) msg[i]); > > return 1; > > } > > > > @@ -363,7 +367,7 @@ do_capture(const char *streamport, FILE *f_log) > > syslog(LOG_INFO, "streaming starts now\n"); > > uint64_t time_last = 0; > > > > - std::unique_ptr<FrameCapture> > > capture(agent.GetBestFrameCapture()); > > + std::unique_ptr<FrameCapture> > > capture(agent.GetBestFrameCapture(client_codecs)); > > if (!capture) > > throw std::runtime_error("cannot find a suitable capture > > system"); > > > > > Otherwise it looks ok to me > > Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> > > Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel