<html><body><div style="font-family: times new roman, new york, times, serif; font-size: 12pt; color: #000000"><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><br class=""><blockquote class=""><div class="">On 15 Nov 2017, at 13:55, Frediano Ziglio <<a href="mailto:fziglio@redhat.com" class="" target="_blank">fziglio@redhat.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><br class="">From: Christophe de Dinechin <<a href="mailto:dinechin@redhat.com" class="" target="_blank">dinechin@redhat.com</a>><br class=""><br class="">Several functional objectives:<br class=""><br class="">1. Be able to share some common options, e.g. fps<br class="">2. Prepare for the possibility to update options on the fly<br class=""><br class="">Also get rid of the null-terminated C++ vector<br class=""><br class="">Signed-off-by: Christophe de Dinechin <<a href="mailto:dinechin@redhat.com" class="" target="_blank">dinechin@redhat.com</a>><br class="">---<br class="">include/spice-streaming-agent/plugin.hpp | 120<br class="">+++++++++++++++++++++++++++----<br class="">m4/spice-compile-warnings.m4 | 2 +<br class="">src/concrete-agent.cpp | 77 ++++++++++++++++----<br class="">src/concrete-agent.hpp | 30 ++++----<br class="">src/mjpeg-fallback.cpp | 90 +++++++++++------------<br class="">5 files changed, 227 insertions(+), 92 deletions(-)<br class=""><br class="">diff --git a/include/spice-streaming-agent/plugin.hpp<br class="">b/include/spice-streaming-agent/plugin.hpp<br class="">index 607fabf..41ad11f 100644<br class="">--- a/include/spice-streaming-agent/plugin.hpp<br class="">+++ b/include/spice-streaming-agent/plugin.hpp<br class="">@@ -8,6 +8,11 @@<br class="">#define SPICE_STREAMING_AGENT_PLUGIN_HPP<br class=""><br class="">#include <spice/enums.h><br class="">+#include <climits><br class="">+#include <sstream><br class="">+#include <string><br class="">+#include <vector><br class="">+<br class=""><br class="">/*!<br class=""> * \file<br class="">@@ -20,6 +25,7 @@<br class="">namespace SpiceStreamingAgent {<br class=""><br class="">class FrameCapture;<br class="">+class Agent;<br class=""><br class="">/*!<br class=""> * Plugin version, only using few bits, schema is 0xMMmm<br class="">@@ -42,13 +48,23 @@ enum Ranks : unsigned {<br class="">};<br class=""><br class="">/*!<br class="">- * Configuration option.<br class="">- * An array of these will be passed to the plugin.<br class="">- * Simply a couple name and value passed as string.<br class="">- * For instance "framerate" and "20".<br class="">+ * Settings that are common to all plugins<br class=""> */<br class="">-struct ConfigureOption<br class="">+struct Settings<br class="">{<br class="">+ unsigned framerate = 30; // Frames per second (1-240)<br class="">+ unsigned quality = 80; // Normalized in 0-100 (100=high)<br class="">+ unsigned avg_bitrate = 3000000; // Target average bitrate in bps<br class="">+ unsigned max_bitrate = 8000000; // Target maximum bitrate<br class="">+};<br class="">+<br class="">+/*!<br class="">+ * Command line option<br class="">+ */<br class="">+struct Option<br class="">+{<br class="">+ Option(const char *name, const char *value)<br class="">+ : name(name), value(value) {}<br class=""> const char *name;<br class=""> const char *value;<br class="">};<br class="">@@ -59,6 +75,9 @@ struct ConfigureOption<br class=""> * A plugin module can register multiple Plugin interfaces to handle<br class=""> * multiple codecs. In this case each Plugin will report data for a<br class=""> * specific codec.<br class="">+ *<br class="">+ * A plugin will typically extend the Settings struct to add its own<br class="">+ * settings.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Not clear how and why.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>Yes, the other patch series for the plugin did not go through, the originating</div><div>email address was rejected by the list server. Working on it :-)</div><br class=""><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""> */<br class="">class Plugin<br class="">{<br class="">@@ -66,7 +85,17 @@ public:<br class=""> /*!<br class=""> * Allows to free the plugin when not needed<br class=""> */<br class="">- virtual ~Plugin() {};<br class="">+ virtual ~Plugin() {}<br class="">+<br class="">+ /*!<br class="">+ * Return the name of the plugin, e.g. for command-line usage<br class="">information<br class="">+ */<br class="">+ virtual const char *Name() = 0;<br class="">+<br class="">+ /*!<br class="">+ * Return usage information for the plug-in, e.g. command-line options<br class="">+ */<br class="">+ virtual const char *Usage() = 0;<br class=""><br class=""> /*!<br class=""> * Request an object for getting frames.<br class="">@@ -89,6 +118,19 @@ public:<br class=""> * Get video codec used to encode last frame<br class=""> */<br class=""> virtual SpiceVideoCodecType VideoCodecType() const=0;<br class="">+<br class="">+ /*!<br class="">+ * Return the concrete Settings for this plugin<br class="">+ */<br class="">+ virtual Settings & PluginSettings() = 0;<br class="">+<br class="">+ /*!<br class="">+ * Apply a given option to the plugin settings.<br class="">+ * \return true if the option is valid, false if it is not recognized<br class="">+ * If there is an error applying the settings, set \arg error.<br class="">+ * If this returns false, agent will try StandardOption.<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">The agent knows its options so does not need to call ApplyOption to</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">know if is a standard option or not.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>I decided to do it the other way round, so that a plugin can do something</div><div>special with a standard option, e.g. special limits on bitrate.</div></div></blockquote><div>Let me clarify one point. I agree the options interface lacks some feature. Changing<br></div><div>them dynamically will be necessary.<br></div><div>On the other hand the interface you are proposing is adding a lot of constrains which<br></div><div>can be hard to bend in the future. As said having a fixed Settings structure is hard to<br></div><div>extend. More that this having a method returning a reference to it force the Plugin<br></div><div>to have a field containing it making it a strong ABI. A workaround would be to</div><div> have a base class for it and allocate from the Agent. But I don't thinkthis will get so far.<br></div><div>It looks that your interface is forcing both Agent and Plugin to handle the Settings<br></div><div>structure. This looks messy to me, the responsibilities should not be shared that<br></div><div>easily. I don't really understand why an option should be really shared.<br></div><div>The agent should propose a value, the Plugin can accept and use, ignore or use<br></div><div>a more sensible data. For instance if I say to a Plugin "I want 9600 bps" the Plugin<br></div><div>should fallback to some more sensible value. I don't see the reason for the Agent to<br></div><div>know the internal value (beside debugging purposes).<br></div><div>The ApplyOption API push each option from the Agent to the Plugin. It looks<br></div><div>like a property settings in some way. But what happens if some values are correlated?<br></div><div>Let suppose I have (not in this case) a width and height options and the object needs<br></div><div>to resize some internal memory based on width and height. Should the resize happens<br></div><div>changing width or height? Or should the object just cache the values, mark as changed<br></div><div>and resize the memory lazily? Would not be better to pass all options I want to change?<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><br class=""><blockquote class=""><div class=""><div style="" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Why not using exceptions instead of a value passed as reference?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>I was concerned about exceptions requiring RTTI information to be</div><div>shared between the agent and the plugin, which I thought might lead</div><div>to linking issues at dlopen time (much like if you leave the Plugin base</div><div>class have non-pure virtual methods).</div><div><br class=""></div><div>Experimentally, that does not appear to be an issue, so I will do</div><div>that in the next iteration.</div></div></blockquote><div>On Unix is a good idea to have the exceptions classes not with restricted<br></div><div>visibility. On Windows C++ is in the system ABI and this is supported by<br></div><div>design.<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><div>(BTW, you might think that having implemented the first “efficient”</div><div>exception handling mechanism, I would naturally be a big proponent</div><div>and user of exceptions. But it’s actually the opposite. I know how</div><div>expensive this stuff is, and how much it still hinders optimizations</div><div>for practical reasons. That probably tends to color my preferences</div><div>towards not using exception unless it’s really in some very slow</div><div>or infrequent execution path.)</div><div><br class=""></div><div><br class=""></div><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">+ */<br class="">+ virtual bool ApplyOption(const Option &o, std::string &error) = 0;<br class="">};<br class=""><br class="">/*!<br class="">@@ -113,24 +155,74 @@ public:<br class=""> * Check if a given plugin version is compatible with this agent<br class=""> * \return true is compatible<br class=""> */<br class="">- virtual bool PluginVersionIsCompatible(unsigned pluginVersion) const=0;<br class="">+ virtual bool PluginVersionIsCompatible(unsigned pluginVersion) const =<br class="">0;<br class=""><br class=""> /*!<br class="">- * Register a plugin in the system.<br class="">+ * Register a plugin in the system, which becomes the owner and should<br class="">delete it.<br class=""> */<br class="">- virtual void Register(Plugin& plugin)=0;<br class="">+ virtual void Register(Plugin *plugin)=0;<br class=""><br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">We should add a style document file.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>The reason I made that change is not style, it is documenting that the target</div><div>takes ownership of it and deletes it (through the shared_ptr vector).</div><div><br class=""></div><div>If you use a reference, it encourages the caller to pass a stack-based plugin,</div><div>expecting that the agent would make a copy or something, e.g. the following</div><div>would seem to be a natural usage:</div><div><br class=""></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>MyPlugin plugin(…);</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>agent->Register(plugin);</div><div><br class=""></div><div>not the rather clumsy:</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>shared_ptr<MyPlugin> plugin = new MyPlugin(…);</div><div><span class="Apple-tab-span" style="white-space:pre"> </span>agent->Register(*plugin);</div><div><br class=""></div><div>So I added a comment stating that the plugin has shared ownership of the pointer</div><div>(instead of stating that the pointer will be deleted). Now the natural usage is</div><div>the most natural way to dynamically allocate the plugin:</div><div><br class=""></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>agent->Register(new MyPlugin(…));</div><div><br class=""></div><div><br class=""></div><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""> /*!<br class="">- * Get options array.<br class="">- * Array is terminated with {nullptr, nullptr}.<br class="">- * Never nullptr.<br class="">- * \todo passing options to entry point instead?<br class="">+ * Apply the standard options to the given settings (base Settings<br class="">class)<br class=""> */<br class="">- virtual const ConfigureOption* Options() const=0;<br class="">+ virtual bool StandardOption(Settings &, const Option &, std::string<br class="">&error) = 0;<br class="">+<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Is not clear this. Is the plugin telling the agent it changed some settings?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Or telling the agent to update the setting structure?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">What the result mean?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>It’s a historical leftover. Initially, the plugin was telling the agent, but then</div><div>I decided it was too dangerous to leave that to the agent, and forgot to remove</div><div>the function. I will remove it.</div><br class=""><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">};<br class=""><br class="">typedef bool PluginInitFunc(SpiceStreamingAgent::Agent* agent);<br class=""><br class="">+/*!<br class="">+ * Helper to get integer values from command-line options<br class="">+ */<br class="">+<br class="">+static inline int IntOption(const Option &opt, std::string &error,<br class="">+ int min=INT_MIN, int max=INT_MAX, bool<br class="">sizeSuffixOK = false)<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Why not Agent::ParseIntOption or something similar?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>I hesitated between the two. That was forcing the plugin to call back into</div><div>the agent.</div><div><br class=""></div><div>One thing I hesitated to do was to add a field to the Plugin:</div><div><br class=""></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>Agent *agent;</div><div><br class=""></div><div>I don’t like globals much. If that’s OK with you, I’d switch to having</div></div></blockquote><div>I don't see any global... and no reason to add a field to Plugin either.<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><div>the option parsing exposed as an agent feature. That also reduces</div><div>the number of standard headers needed for the interface.</div><div>Definitely better.</div><br class=""><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">+{<br class="">+ std::string name = opt.name;<br class="">+ std::string value = opt.value;<br class="">+ std::ostringstream err;<br class="">+ std::istringstream input(value);<br class="">+ int result = 0;<br class="">+ input >> result;<br class="">+<br class="">+ if (!input.fail() && !input.eof() && sizeSuffixOK) {<br class="">+ std::string suffix;<br class="">+ input >> suffix;<br class="">+ bool ok = false;<br class="">+ if (!input.fail() && suffix.length() == 1) {<br class="">+ switch (suffix[0]) {<br class="">+ case 'b': case 'B': ok = true; break;<br class="">+ case 'k': case 'K': ok = true; result *= 1000; break;<br class="">+ case 'm': case 'M': ok = true; result *= 1000000; break;<br class="">+ default: break;<br class="">+ }<br class="">+ }<br class="">+ if (!ok) {<br class="">+ err << "Unknown number suffix " << suffix<br class="">+ << " for " << name << "\n";<br class="">+ error = err.str();<br class="">+ }<br class="">+ }<br class="">+<br class="">+ if (input.fail()) {<br class="">+ err << "Value " << value << " for " << name << " is not a number\n";<br class="">+ error = err.str();<br class="">+ }<br class="">+ if (!input.eof()) {<br class="">+ err << "Value " << value << " for " << name << " does not end like<br class="">an integer\n";<br class="">+ error = err.str();<br class="">+ }<br class="">+ if (result < min || result > max) {<br class="">+ err << "The value " << value << " for " << name<br class="">+ << " is out of range (must be in " << min << ".." << max <<<br class="">")\n";<br class="">+ error = err.str(); // May actually combine an earlier error<br class="">+ result = (min + max) / 2; // Give a value acceptable by caller<br class="">+ }<br class="">+<br class="">+ return result;<br class="">+}<br class="">+<br class="">+<br class="">}<br class=""><br class="">#ifndef SPICE_STREAMING_AGENT_PROGRAM<br class="">diff --git a/m4/spice-compile-warnings.m4 b/m4/spice-compile-warnings.m4<br class="">index 66d7179..9e8bb6e 100644<br class="">--- a/m4/spice-compile-warnings.m4<br class="">+++ b/m4/spice-compile-warnings.m4<br class="">@@ -86,6 +86,8 @@ AC_DEFUN([SPICE_COMPILE_WARNINGS],[<br class=""> dontwarn="$dontwarn -Wpointer-sign"<br class=""> dontwarn="$dontwarn -Wpointer-to-int-cast"<br class=""> dontwarn="$dontwarn -Wstrict-prototypes"<br class="">+ dontwarn="$dontwarn -Wsuggest-final-types"<br class="">+ dontwarn="$dontwarn -Wsuggest-final-methods"<br class=""><br class=""> # We want to enable these, but need to sort out the<br class=""> # decl mess with gtk/generated_*.c<br class="">diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp<br class="">index 192054a..59f11b2 100644<br class="">--- a/src/concrete-agent.cpp<br class="">+++ b/src/concrete-agent.cpp<br class="">@@ -9,6 +9,7 @@<br class="">#include <syslog.h><br class="">#include <glob.h><br class="">#include <dlfcn.h><br class="">+#include <mutex><br class=""><br class="">#include "concrete-agent.hpp"<br class="">#include "static-plugin.hpp"<br class="">@@ -28,7 +29,11 @@ static inline unsigned MinorVersion(unsigned version)<br class=""><br class="">ConcreteAgent::ConcreteAgent()<br class="">{<br class="">- options.push_back(ConcreteConfigureOption(nullptr, nullptr));<br class="">+}<br class="">+<br class="">+void ConcreteAgent::Register(Plugin *plugin)<br class="">+{<br class="">+ plugins.push_back(shared_ptr<Plugin>(plugin));<br class="">}<br class=""><br class="">bool ConcreteAgent::PluginVersionIsCompatible(unsigned pluginVersion) const<br class="">@@ -38,22 +43,34 @@ bool ConcreteAgent::PluginVersionIsCompatible(unsigned<br class="">pluginVersion) const<br class=""> MinorVersion(version) >= MinorVersion(pluginVersion);<br class="">}<br class=""><br class="">-void ConcreteAgent::Register(Plugin& plugin)<br class="">+bool ConcreteAgent::StandardOption(Settings &settings,<br class="">+ const Option &option,<br class="">+ string &error)<br class="">{<br class="">- plugins.push_back(shared_ptr<Plugin>(&plugin));<br class="">-}<br class="">+ string name = option.name;<br class="">+ if (name == "framerate" || name == "fps") {<br class="">+ settings.framerate = IntOption(option, error, 1, 240);<br class="">+ return true;<br class="">+ }<br class="">+ if (name == "quality") {<br class="">+ settings.quality = IntOption(option, error, 0, 100);<br class="">+ return true;<br class="">+ }<br class="">+ if (name == "avg_bitrate" || name == "bitrate") {<br class="">+ settings.avg_bitrate = IntOption(option, error, 10000, 32000000,<br class="">true);<br class="">+ return true;<br class="">+ }<br class="">+ if (name == "max_bitrate") {<br class="">+ settings.max_bitrate = IntOption(option, error, 10000, 32000000,<br class="">true);<br class="">+ return true;<br class="">+ }<br class=""><br class="">-const ConfigureOption* ConcreteAgent::Options() const<br class="">-{<br class="">- static_assert(sizeof(ConcreteConfigureOption) ==<br class="">sizeof(ConfigureOption),<br class="">- "ConcreteConfigureOption should be binary compatible with<br class="">ConfigureOption");<br class="">- return static_cast<const ConfigureOption*>(&options[0]);<br class="">+ return false; // Unrecognized option<br class="">}<br class=""><br class="">void ConcreteAgent::AddOption(const char *name, const char *value)<br class="">{<br class="">- // insert before the last {nullptr, nullptr} value<br class="">- options.insert(--options.end(), ConcreteConfigureOption(name, value));<br class="">+ options.push_back(Option(name, value));<br class="">}<br class=""><br class="">void ConcreteAgent::LoadPlugins(const char *directory)<br class="">@@ -81,7 +98,8 @@ void ConcreteAgent::LoadPlugin(const char *plugin_filename)<br class="">{<br class=""> void *dl = dlopen(plugin_filename, RTLD_LOCAL|RTLD_NOW);<br class=""> if (!dl) {<br class="">- syslog(LOG_ERR, "error loading plugin %s", plugin_filename);<br class="">+ syslog(LOG_ERR, "error loading plugin %s: %s",<br class="">+ plugin_filename, dlerror());<br class=""> return;<br class=""> }<br class=""><br class="">@@ -103,7 +121,7 @@ FrameCapture *ConcreteAgent::GetBestFrameCapture()<br class=""> vector<pair<unsigned, shared_ptr<Plugin>>> sorted_plugins;<br class=""><br class=""> // sort plugins base on ranking, reverse order<br class="">- for (const auto& plugin: plugins) {<br class="">+ for (auto& plugin: plugins) {<br class=""> sorted_plugins.push_back(make_pair(plugin->Rank(), plugin));<br class=""> }<br class=""> sort(sorted_plugins.rbegin(), sorted_plugins.rend());<br class="">@@ -113,6 +131,7 @@ FrameCapture *ConcreteAgent::GetBestFrameCapture()<br class=""> if (plugin.first == DontUse) {<br class=""> break;<br class=""> }<br class="">+ ApplyOptions(plugin.second.get());<br class=""> FrameCapture *capture = plugin.second->CreateCapture();<br class=""> if (capture) {<br class=""> return capture;<br class="">@@ -120,3 +139,35 @@ FrameCapture *ConcreteAgent::GetBestFrameCapture()<br class=""> }<br class=""> return nullptr;<br class="">}<br class="">+<br class="">+void ConcreteAgent::ApplyOptions(Plugin *plugin)<br class="">+{<br class="">+ bool usage = false;<br class="">+ for (const auto &opt : options) {<br class="">+ string error;<br class="">+ bool known = plugin->ApplyOption(opt, error);<br class="">+ if (!known) {<br class="">+ Settings &settings = plugin->PluginSettings();<br class="">+ known = StandardOption(settings, opt, error);<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Can't the agent check for standard before?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>As explained above, the idea is that the agent might have special</div><div>ways to deal with standard options, e.g. restrictions on the range.</div></div></blockquote><div>I the agent restrict the range should craft a better option value and pass to the<br></div><div>Plugin. If the Plugin has a better value should just use.<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><br class=""><blockquote class=""><div class=""><div style="" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">How the plugin knows the agent changed the settings?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>Does it care? Right now, I am not testing nor reporting if the setting was modified.</div></div></blockquote><div>So, the Agent can change internal settings of the Plugin and the Plugin does not need</div><div>to know?<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><br class=""><blockquote class=""><div class=""><div style="" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">And why StandardOption is public if called only by the agent?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>As written above, leftover from code evolution, I’ll remove it.</div></div></blockquote><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><br class=""><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">+ }<br class="">+ if (!known) {<br class="">+ syslog(LOG_ERR, "Option %s not recognized by plugin %s",<br class="">+ opt.name, plugin->Name());<br class="">+ usage = true;<br class="">+ }<br class="">+ if (error != "") {<br class="">+ syslog(LOG_ERR, "Plugin %s did not accept setting %s=%s: %s",<br class="">+ plugin->Name(), opt.name, opt.value, error.c_str());<br class="">+ usage = true;<br class="">+ }<br class="">+ }<br class="">+ if (usage)<br class="">+ {<br class="">+ static std:: once_flag once;<br class="">+ std::call_once(once, [plugin]()<br class="">+ {<br class="">+ const char *usage = plugin->Usage();<br class="">+ syslog(LOG_ERR, "Usage: for plugin %s: %s", plugin->Name(),<br class="">usage);<br class="">+ });<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">This way the error/warning depends on the order of the plugins.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>Hmm. You are right. But if a given plugin receives some incorrect input</div><div>repeatedly, I don’t want to flood the syslog. We already have the error</div><div>message that tells what option failed.</div></div></blockquote><div>But is neither an error nor a warning.<br></div><div>If pluginA supports optionA and pluginB supports optionB to avoid these message<br></div><div>I cannot use neither optionA nor optionB that is you basically cannot have options<br></div><div>for the plugins but only standard options.<br></div><div>Unless you want to have specific plugin options, in this case you can say that if<br></div><div>optionX for pluginX is not supported by pluginX you give an error/warning<br></div><div>while optionX for pluginX is ignored by pluginY.<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><div><br class=""></div><div>Thinking about it, I think showing usage information here is wrong,</div><div>I’ll remove it. The order of the messages is the order of the plugins</div><div>which is not consistent, but that’s another debate.</div><br class=""><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">+ }<br class="">+}<br class="">diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp<br class="">index 828368b..b3d4e06 100644<br class="">--- a/src/concrete-agent.hpp<br class="">+++ b/src/concrete-agent.hpp<br class="">@@ -12,33 +12,33 @@<br class=""><br class="">namespace SpiceStreamingAgent {<br class=""><br class="">-struct ConcreteConfigureOption: ConfigureOption<br class="">-{<br class="">- ConcreteConfigureOption(const char *name, const char *value)<br class="">- {<br class="">- this->name = name;<br class="">- this->value = value;<br class="">- }<br class="">-};<br class="">-<br class="">class ConcreteAgent final : public Agent<br class="">{<br class="">public:<br class=""> ConcreteAgent();<br class="">+<br class="">+public:<br class="">+ // Implementation of the Agent class virtual methods<br class=""> unsigned Version() const override {<br class=""> return PluginVersion;<br class=""> }<br class="">- void Register(Plugin& plugin) override;<br class="">- const ConfigureOption* Options() const override;<br class="">- void LoadPlugins(const char *directory);<br class="">- // pointer must remain valid<br class="">+ void Register(Plugin *plugin) override;<br class="">+ bool PluginVersionIsCompatible(unsigned pluginVersion) const override;<br class="">+ bool StandardOption(Settings &settings,<br class="">+ const Option &option,<br class="">+ std::string &error) override;<br class="">+<br class="">+public:<br class="">+ // Interface used by the main agent program<br class=""> void AddOption(const char *name, const char *value);<br class="">+ void LoadPlugins(const char *directory);<br class="">+ void ApplyOptions(Plugin *plugin);<br class=""></blockquote><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Why this is public?</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Is the program dealing directly with plugins? Should not.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></div></blockquote><div><br class=""></div><div>You are right.</div></div></blockquote><div>Mumble... maybe we should have some documentation on responsibility.<br></div><blockquote style="border-left:2px solid #1010FF;margin-left:5px;padding-left:5px;color:#000;font-weight:normal;font-style:normal;text-decoration:none;font-family:Helvetica,Arial,sans-serif;font-size:12pt;"><div><br class=""><blockquote class=""><div class=""><div style="" class=""><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""> FrameCapture *GetBestFrameCapture();<br class="">- bool PluginVersionIsCompatible(unsigned pluginVersion) const override;<br class="">+<br class="">private:<br class=""> void LoadPlugin(const char *plugin_filename);<br class=""> std::vector<std::shared_ptr<Plugin>> plugins;<br class="">- std::vector<ConcreteConfigureOption> options;<br class="">+ std::vector<Option> options;<br class="">};<br class=""><br class="">}<br class="">diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp<br class="">index f41e68a..37df01a 100644<br class="">--- a/src/mjpeg-fallback.cpp<br class="">+++ b/src/mjpeg-fallback.cpp<br class="">@@ -41,16 +41,11 @@ static inline uint64_t get_time()<br class="">}<br class=""><br class="">namespace {<br class="">-struct MjpegSettings<br class="">-{<br class="">- int fps;<br class="">- int quality;<br class="">-};<br class=""><br class="">class MjpegFrameCapture final: public FrameCapture<br class="">{<br class="">public:<br class="">- MjpegFrameCapture(const MjpegSettings &settings);<br class="">+ MjpegFrameCapture(Settings &settings);<br class=""> ~MjpegFrameCapture();<br class=""> FrameInfo CaptureFrame() override;<br class=""> void Reset() override;<br class="">@@ -58,8 +53,8 @@ public:<br class=""> return SPICE_VIDEO_CODEC_TYPE_MJPEG;<br class=""> }<br class="">private:<br class="">- MjpegSettings settings;<br class=""> Display *dpy;<br class="">+ Settings &settings;<br class=""><br class=""> vector<uint8_t> frame;<br class=""><br class="">@@ -72,19 +67,20 @@ private:<br class="">class MjpegPlugin final: public Plugin<br class="">{<br class="">public:<br class="">+ virtual const char *Name() override;<br class="">+ virtual const char *Usage() override;<br class=""> FrameCapture *CreateCapture() override;<br class=""> unsigned Rank() override;<br class="">- void ParseOptions(const ConfigureOption *options);<br class="">- SpiceVideoCodecType VideoCodecType() const {<br class="">- return SPICE_VIDEO_CODEC_TYPE_MJPEG;<br class="">- }<br class="">+ SpiceVideoCodecType VideoCodecType() const override;<br class="">+ Settings &PluginSettings() override;<br class="">+ bool ApplyOption(const Option &o, string &error) override;<br class="">private:<br class="">- MjpegSettings settings = { 10, 80 };<br class="">+ Settings settings;<br class="">};<br class="">}<br class=""><br class="">-MjpegFrameCapture::MjpegFrameCapture(const MjpegSettings& settings):<br class="">- settings(settings)<br class="">+MjpegFrameCapture::MjpegFrameCapture(Settings &settings)<br class="">+ : settings(settings)<br class="">{<br class=""> dpy = XOpenDisplay(NULL);<br class=""> if (!dpy)<br class="">@@ -111,7 +107,7 @@ FrameInfo MjpegFrameCapture::CaptureFrame()<br class=""> if (last_time == 0) {<br class=""> last_time = now;<br class=""> } else {<br class="">- const uint64_t delta = 1000000000u / settings.fps;<br class="">+ const uint64_t delta = 1000000000u / settings.framerate;<br class=""> if (now >= last_time + delta) {<br class=""> last_time = now;<br class=""> } else {<br class="">@@ -148,13 +144,13 @@ FrameInfo MjpegFrameCapture::CaptureFrame()<br class=""><br class=""> int format = ZPixmap;<br class=""> // TODO handle errors<br class="">- XImage *image = XGetImage(dpy, win, win_info.x, win_info.y,<br class="">+ XImage *image = XGetImage(dpy, win, win_info.x, win_info.y,<br class=""> win_info.width, win_info.height, AllPlanes,<br class=""> format);<br class=""><br class=""> // TODO handle errors<br class=""> // TODO multiple formats (only 32 bit)<br class="">- write_JPEG_file(frame, settings.quality, (uint8_t*) image->data,<br class="">- image->width, image->height);<br class="">+ write_JPEG_file(frame, settings.quality,<br class="">+ (uint8_t*) image->data, image->width, image->height);<br class=""><br class=""> image->f.destroy_image(image);<br class=""><br class="">@@ -166,6 +162,18 @@ FrameInfo MjpegFrameCapture::CaptureFrame()<br class=""> return info;<br class="">}<br class=""><br class="">+const char *MjpegPlugin::Name()<br class="">+{<br class="">+ return "MJPEG";<br class="">+}<br class="">+<br class="">+const char *MjpegPlugin::Usage()<br class="">+{<br class="">+ return<br class="">+ "quality = [0-100] Set picture quality\n"<br class="">+ "framerate = [1-240] Set number of frames per second\n";<br class="">+}<br class="">+<br class="">FrameCapture *MjpegPlugin::CreateCapture()<br class="">{<br class=""> return new MjpegFrameCapture(settings);<br class="">@@ -176,33 +184,20 @@ unsigned MjpegPlugin::Rank()<br class=""> return FallBackMin;<br class="">}<br class=""><br class="">-void MjpegPlugin::ParseOptions(const ConfigureOption *options)<br class="">+SpiceVideoCodecType MjpegPlugin::VideoCodecType() const<br class="">{<br class="">-#define arg_error(...) syslog(LOG_ERR, ## __VA_ARGS__);<br class="">-<br class="">- for (; options->name; ++options) {<br class="">- const char *name = options->name;<br class="">- const char *value = options->value;<br class="">-<br class="">- if (strcmp(name, "framerate") == 0) {<br class="">- int val = atoi(value);<br class="">- if (val > 0) {<br class="">- settings.fps = val;<br class="">- }<br class="">- else {<br class="">- arg_error("wrong framerate arg %s\n", value);<br class="">- }<br class="">- }<br class="">- if (strcmp(name, "mjpeg.quality") == 0) {<br class="">- int val = atoi(value);<br class="">- if (val > 0) {<br class="">- settings.quality = val;<br class="">- }<br class="">- else {<br class="">- arg_error("wrong mjpeg.quality arg %s\n", value);<br class="">- }<br class="">- }<br class="">- }<br class="">+ return SPICE_VIDEO_CODEC_TYPE_MJPEG;<br class="">+}<br class="">+<br class="">+Settings &MjpegPlugin::PluginSettings()<br class="">+{<br class="">+ return settings;<br class="">+}<br class="">+<br class="">+bool MjpegPlugin::ApplyOption(const Option &o, string &error)<br class="">+{<br class="">+ // This plugin only relies on base options<br class="">+ return false;<br class="">}<br class=""><br class="">static bool<br class="">@@ -211,12 +206,7 @@ mjpeg_plugin_init(Agent* agent)<br class=""> if (agent->Version() != PluginVersion)<br class=""> return false;<br class=""><br class="">- std::unique_ptr<MjpegPlugin> plugin(new MjpegPlugin());<br class="">-<br class="">- plugin->ParseOptions(agent->Options());<br class="">-<br class="">- agent->Register(*plugin.release());<br class="">-<br class="">+ agent->Register(new MjpegPlugin);<br class=""> return true;<br class="">}<br class=""></blockquote></div></div></blockquote></div></blockquote><div>Frediano</div><div><br></div></div></body></html>