<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""> <div> <blockquote type="cite" class=""><div class="">On 14 Nov 2017, at 16:41, Frediano Ziglio <<a href="mailto:fziglio@redhat.com" class="">fziglio@redhat.com</a>> wrote:</div> <div class=""><div style="" class=""><blockquote type="cite" 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=""> From: Christophe de Dinechin <<a href="mailto:dinechin@redhat.com" class="">dinechin@redhat.com</a>> Several functional objectives: 1. Be able to share some common options, e.g. fps 2. Prepare for the possibility to update options on the fly Also get rid of the null-terminated C++ vector Signed-off-by: Christophe de Dinechin <<a href="mailto:dinechin@redhat.com" class="">dinechin@redhat.com</a>> </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 change is neither ABI not API compatible. Why?<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> </div><div>Compatible with what? We are still building the very first versions of this agent,</div><div>and nobody but us has tested it yet.</div><div> </div><div>So I think it’s still the right time to fix things that are broken. In particular the fact</div><div>that the curent API does not make it possible to adjust settings on the fly.</div><div>Half of the experiment I have been doing can’t be done with the curent API.</div> <blockquote type="cite" 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 type="cite" 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="">--- include/spice-streaming-agent/plugin.hpp | 120 +++++++++++++++++++++++++++---- m4/spice-compile-warnings.m4 | 2 + src/concrete-agent.cpp | 77 ++++++++++++++++---- src/concrete-agent.hpp | 30 ++++---- src/mjpeg-fallback.cpp | 90 +++++++++++------------ 5 files changed, 227 insertions(+), 92 deletions(-) diff --git a/include/spice-streaming-agent/plugin.hpp b/include/spice-streaming-agent/plugin.hpp index 607fabf..41ad11f 100644 --- a/include/spice-streaming-agent/plugin.hpp +++ b/include/spice-streaming-agent/plugin.hpp @@ -8,6 +8,11 @@ #define SPICE_STREAMING_AGENT_PLUGIN_HPP #include <spice/enums.h> +#include <climits> +#include <sstream> +#include <string> +#include <vector> + /*! * \file @@ -20,6 +25,7 @@ namespace SpiceStreamingAgent { class FrameCapture; +class Agent; /*! * Plugin version, only using few bits, schema is 0xMMmm @@ -42,13 +48,23 @@ enum Ranks : unsigned { }; /*! - * Configuration option. - * An array of these will be passed to the plugin. - * Simply a couple name and value passed as string. - * For instance "framerate" and "20". + * Settings that are common to all plugins */ -struct ConfigureOption +struct Settings { + unsigned framerate = 30; // Frames per second (1-240) + unsigned quality = 80; // Normalized in 0-100 (100=high) + unsigned avg_bitrate = 3000000; // Target average bitrate in bps + unsigned max_bitrate = 8000000; // Target maximum bitrate +}; + </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="">How are you going to extend this?<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> </div><div>If we see other shared settings, now is a good time to think about them.</div><div>Later additions to these settings would require an ABI break.</div><div> </div><div>(Of note, we have the same issue with the command line options too)</div><div> </div><blockquote type="cite" 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 type="cite" 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="">+/*! + * Command line option + */ +struct Option +{ + Option(const char *name, const char *value) + : name(name), value(value) {} const char *name; const char *value; }; @@ -59,6 +75,9 @@ struct ConfigureOption * A plugin module can register multiple Plugin interfaces to handle * multiple codecs. In this case each Plugin will report data for a * specific codec. + * + * A plugin will typically extend the Settings struct to add its own + * settings. */ class Plugin { @@ -66,7 +85,17 @@ public: /*! * Allows to free the plugin when not needed */ - virtual ~Plugin() {}; + virtual ~Plugin() {} + + /*! + * Return the name of the plugin, e.g. for command-line usage information + */ + virtual const char *Name() = 0; + + /*! + * Return usage information for the plug-in, e.g. command-line options + */ + virtual const char *Usage() = 0; /*! * Request an object for getting frames. @@ -89,6 +118,19 @@ public: * Get video codec used to encode last frame */ virtual SpiceVideoCodecType VideoCodecType() const=0; + + /*! + * Return the concrete Settings for this plugin </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="">For which usage? The result is not const, should we expect to<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="">change them? How to notify the plugin of changes?<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> </div><div>See actual call. The usage is to let the agent manage options for</div><div>plugins without knowing the details of their settings.</div><div> </div><div>Ideally, I would have preferred a base class to deal with that, but</div><div>that does not work, because the base class code is in the agent and</div><div>the derived class in dynamically loaded plugins, so dlopen fails.</div> <blockquote type="cite" 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 type="cite" 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="">+ */ + virtual Settings & PluginSettings() = 0; + + /*! + * Apply a given option to the plugin settings. + * \return true if the option is valid, false if it is not recognized + * If there is an error applying the settings, set \arg error. + * If this returns false, agent will try StandardOption. + */ + virtual bool ApplyOption(const Option &o, std::string &error) = 0; }; /*! @@ -113,24 +155,74 @@ public: * Check if a given plugin version is compatible with this agent * \return true is compatible */ - virtual bool PluginVersionIsCompatible(unsigned pluginVersion) const=0; + virtual bool PluginVersionIsCompatible(unsigned pluginVersion) const = 0; /*! - * Register a plugin in the system. + * Register a plugin in the system, which becomes the owner and should delete it. */ - virtual void Register(Plugin& plugin)=0; + virtual void Register(Plugin *plugin)=0; /*! - * Get options array. - * Array is terminated with {nullptr, nullptr}. - * Never nullptr. - * \todo passing options to entry point instead? + * Apply the standard options to the given settings (base Settings class) */ - virtual const ConfigureOption* Options() const=0; + virtual bool StandardOption(Settings &, const Option &, std::string &error) = 0; + }; typedef bool PluginInitFunc(SpiceStreamingAgent::Agent* agent); +/*! + * Helper to get integer values from command-line options + */ + +static inline int IntOption(const Option &opt, std::string &error, + int min=INT_MIN, int max=INT_MAX, bool sizeSuffixOK = false) +{ + std::string name = opt.name; + std::string value = opt.value; + std::ostringstream err; + std::istringstream input(value); + int result = 0; + input >> result; + + if (!input.fail() && !input.eof() && sizeSuffixOK) { + std::string suffix; + input >> suffix; + bool ok = false; + if (!input.fail() && suffix.length() == 1) { + switch (suffix[0]) { + case 'b': case 'B': ok = true; break; + case 'k': case 'K': ok = true; result *= 1000; break; + case 'm': case 'M': ok = true; result *= 1000000; break; + default: break; + } + } + if (!ok) { + err << "Unknown number suffix " << suffix + << " for " << name << "\n"; + error = err.str(); + } + } + + if (input.fail()) { + err << "Value " << value << " for " << name << " is not a number\n"; + error = err.str(); + } + if (!input.eof()) { + err << "Value " << value << " for " << name << " does not end like an integer\n"; + error = err.str(); + } + if (result < min || result > max) { + err << "The value " << value << " for " << name + << " is out of range (must be in " << min << ".." << max << ")\n"; + error = err.str(); // May actually combine an earlier error + result = (min + max) / 2; // Give a value acceptable by caller + } + + return result; +} + + } #ifndef SPICE_STREAMING_AGENT_PROGRAM diff --git a/m4/spice-compile-warnings.m4 b/m4/spice-compile-warnings.m4 index 66d7179..9e8bb6e 100644 --- a/m4/spice-compile-warnings.m4 +++ b/m4/spice-compile-warnings.m4 @@ -86,6 +86,8 @@ AC_DEFUN([SPICE_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wpointer-sign" dontwarn="$dontwarn -Wpointer-to-int-cast" dontwarn="$dontwarn -Wstrict-prototypes" + dontwarn="$dontwarn -Wsuggest-final-types" + dontwarn="$dontwarn -Wsuggest-final-methods" # We want to enable these, but need to sort out the # decl mess with gtk/generated_*.c diff --git a/src/concrete-agent.cpp b/src/concrete-agent.cpp index 192054a..59f11b2 100644 --- a/src/concrete-agent.cpp +++ b/src/concrete-agent.cpp @@ -9,6 +9,7 @@ #include <syslog.h> #include <glob.h> #include <dlfcn.h> +#include <mutex> #include "concrete-agent.hpp" #include "static-plugin.hpp" @@ -28,7 +29,11 @@ static inline unsigned MinorVersion(unsigned version) ConcreteAgent::ConcreteAgent() { - options.push_back(ConcreteConfigureOption(nullptr, nullptr)); +} + +void ConcreteAgent::Register(Plugin *plugin) +{ + plugins.push_back(shared_ptr<Plugin>(plugin)); } bool ConcreteAgent::PluginVersionIsCompatible(unsigned pluginVersion) const @@ -38,22 +43,34 @@ bool ConcreteAgent::PluginVersionIsCompatible(unsigned pluginVersion) const MinorVersion(version) >= MinorVersion(pluginVersion); } -void ConcreteAgent::Register(Plugin& plugin) +bool ConcreteAgent::StandardOption(Settings &settings, + const Option &option, + string &error) { - plugins.push_back(shared_ptr<Plugin>(&plugin)); -} + string name = option.name; + if (name == "framerate" || name == "fps") { + settings.framerate = IntOption(option, error, 1, 240); + return true; + } + if (name == "quality") { + settings.quality = IntOption(option, error, 0, 100); + return true; + } + if (name == "avg_bitrate" || name == "bitrate") { + settings.avg_bitrate = IntOption(option, error, 10000, 32000000, true); + return true; + } + if (name == "max_bitrate") { + settings.max_bitrate = IntOption(option, error, 10000, 32000000, true); + return true; + } -const ConfigureOption* ConcreteAgent::Options() const -{ - static_assert(sizeof(ConcreteConfigureOption) == sizeof(ConfigureOption), - "ConcreteConfigureOption should be binary compatible with ConfigureOption"); - return static_cast<const ConfigureOption*>(&options[0]); + return false; // Unrecognized option } void ConcreteAgent::AddOption(const char *name, const char *value) { - // insert before the last {nullptr, nullptr} value - options.insert(--options.end(), ConcreteConfigureOption(name, value)); + options.push_back(Option(name, value)); } void ConcreteAgent::LoadPlugins(const char *directory) @@ -81,7 +98,8 @@ void ConcreteAgent::LoadPlugin(const char *plugin_filename) { void *dl = dlopen(plugin_filename, RTLD_LOCAL|RTLD_NOW); if (!dl) { - syslog(LOG_ERR, "error loading plugin %s", plugin_filename); + syslog(LOG_ERR, "error loading plugin %s: %s", + plugin_filename, dlerror()); return; } @@ -103,7 +121,7 @@ FrameCapture *ConcreteAgent::GetBestFrameCapture() vector<pair<unsigned, shared_ptr<Plugin>>> sorted_plugins; // sort plugins base on ranking, reverse order - for (const auto& plugin: plugins) { + for (auto& plugin: plugins) { sorted_plugins.push_back(make_pair(plugin->Rank(), plugin)); } sort(sorted_plugins.rbegin(), sorted_plugins.rend()); @@ -113,6 +131,7 @@ FrameCapture *ConcreteAgent::GetBestFrameCapture() if (plugin.first == DontUse) { break; } + ApplyOptions(plugin.second.get()); FrameCapture *capture = plugin.second->CreateCapture(); if (capture) { return capture; @@ -120,3 +139,35 @@ FrameCapture *ConcreteAgent::GetBestFrameCapture() } return nullptr; } + +void ConcreteAgent::ApplyOptions(Plugin *plugin) +{ + bool usage = false; + for (const auto &opt : options) { + string error; + bool known = plugin->ApplyOption(opt, error); + if (!known) { + Settings &settings = plugin->PluginSettings(); + known = StandardOption(settings, opt, error); + } + if (!known) { + syslog(LOG_ERR, "Option %s not recognized by plugin %s", + opt.name, plugin->Name()); + usage = true; + } + if (error != "") { + syslog(LOG_ERR, "Plugin %s did not accept setting %s=%s: %s", + plugin->Name(), opt.name, opt.value, error.c_str()); + usage = true; + } + } </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="">How do you deal with options supported by another plugin but not this<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="">and not standard? Looks like like an error. I think should be ignored.<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> </div><div>This is why it goes to syslog but does not throw. Maybe log a warning instead</div><div>of an error?</div> <blockquote type="cite" 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 type="cite" 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="">+ if (usage) + { + static std:: once_flag once; + std::call_once(once, [plugin]() + { + const char *usage = plugin->Usage(); + syslog(LOG_ERR, "Usage: for plugin %s: %s", plugin->Name(), usage); + }); + } +} diff --git a/src/concrete-agent.hpp b/src/concrete-agent.hpp index 828368b..b3d4e06 100644 --- a/src/concrete-agent.hpp +++ b/src/concrete-agent.hpp @@ -12,33 +12,33 @@ namespace SpiceStreamingAgent { -struct ConcreteConfigureOption: ConfigureOption -{ - ConcreteConfigureOption(const char *name, const char *value) - { - this->name = name; - this->value = value; - } -}; - class ConcreteAgent final : public Agent { public: ConcreteAgent(); + +public: + // Implementation of the Agent class virtual methods unsigned Version() const override { return PluginVersion; } - void Register(Plugin& plugin) override; - const ConfigureOption* Options() const override; - void LoadPlugins(const char *directory); - // pointer must remain valid + void Register(Plugin *plugin) override; + bool PluginVersionIsCompatible(unsigned pluginVersion) const override; + bool StandardOption(Settings &settings, + const Option &option, + std::string &error) override; + +public: + // Interface used by the main agent program void AddOption(const char *name, const char *value); + void LoadPlugins(const char *directory); + void ApplyOptions(Plugin *plugin); FrameCapture *GetBestFrameCapture(); - bool PluginVersionIsCompatible(unsigned pluginVersion) const override; + private: void LoadPlugin(const char *plugin_filename); std::vector<std::shared_ptr<Plugin>> plugins; - std::vector<ConcreteConfigureOption> options; + std::vector<Option> options; }; } diff --git a/src/mjpeg-fallback.cpp b/src/mjpeg-fallback.cpp index f41e68a..37df01a 100644 --- a/src/mjpeg-fallback.cpp +++ b/src/mjpeg-fallback.cpp @@ -41,16 +41,11 @@ static inline uint64_t get_time() } namespace { -struct MjpegSettings -{ - int fps; - int quality; -}; class MjpegFrameCapture final: public FrameCapture { public: - MjpegFrameCapture(const MjpegSettings &settings); + MjpegFrameCapture(Settings &settings); ~MjpegFrameCapture(); FrameInfo CaptureFrame() override; void Reset() override; @@ -58,8 +53,8 @@ public: return SPICE_VIDEO_CODEC_TYPE_MJPEG; } private: - MjpegSettings settings; Display *dpy; + Settings &settings; vector<uint8_t> frame; @@ -72,19 +67,20 @@ private: class MjpegPlugin final: public Plugin { public: + virtual const char *Name() override; + virtual const char *Usage() override; FrameCapture *CreateCapture() override; unsigned Rank() override; - void ParseOptions(const ConfigureOption *options); - SpiceVideoCodecType VideoCodecType() const { - return SPICE_VIDEO_CODEC_TYPE_MJPEG; - } + SpiceVideoCodecType VideoCodecType() const override; + Settings &PluginSettings() override; + bool ApplyOption(const Option &o, string &error) override; private: - MjpegSettings settings = { 10, 80 }; + Settings settings; }; } -MjpegFrameCapture::MjpegFrameCapture(const MjpegSettings& settings): - settings(settings) +MjpegFrameCapture::MjpegFrameCapture(Settings &settings) + : settings(settings) { dpy = XOpenDisplay(NULL); if (!dpy) @@ -111,7 +107,7 @@ FrameInfo MjpegFrameCapture::CaptureFrame() if (last_time == 0) { last_time = now; } else { - const uint64_t delta = 1000000000u / settings.fps; + const uint64_t delta = 1000000000u / settings.framerate; if (now >= last_time + delta) { last_time = now; } else { @@ -148,13 +144,13 @@ FrameInfo MjpegFrameCapture::CaptureFrame() int format = ZPixmap; // TODO handle errors - XImage *image = XGetImage(dpy, win, win_info.x, win_info.y, + XImage *image = XGetImage(dpy, win, win_info.x, win_info.y, win_info.width, win_info.height, AllPlanes, format); // TODO handle errors // TODO multiple formats (only 32 bit) - write_JPEG_file(frame, settings.quality, (uint8_t*) image->data, - image->width, image->height); + write_JPEG_file(frame, settings.quality, + (uint8_t*) image->data, image->width, image->height); image->f.destroy_image(image); </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="">don't get changes here. Just spaces?<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> </div>Yes. Grouping all image parameters together as a result of not-published</div><div>intermediate stages on this code.</div><div> <blockquote type="cite" 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 type="cite" 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="">@@ -166,6 +162,18 @@ FrameInfo MjpegFrameCapture::CaptureFrame() return info; } +const char *MjpegPlugin::Name() +{ + return "MJPEG"; +} + </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="">Yes, name is really helpful.<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=""><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 type="cite" 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="">+const char *MjpegPlugin::Usage() +{ + return + "quality = [0-100] Set picture quality\n" + "framerate = [1-240] Set number of frames per second\n"; +} + </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="">Wondering about the indentation here ("quality" and " = [").<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="">There should be a standard maybe. Or a structured way to return<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="">these informations.<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> </div><div>I considered returning an array and letting the caller do the formatting.</div><div>And I think you are right, it’s the way to go. I’ll do that.</div> <blockquote type="cite" 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 type="cite" 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 *MjpegPlugin::CreateCapture() { return new MjpegFrameCapture(settings); @@ -176,33 +184,20 @@ unsigned MjpegPlugin::Rank() return FallBackMin; } -void MjpegPlugin::ParseOptions(const ConfigureOption *options) +SpiceVideoCodecType MjpegPlugin::VideoCodecType() const { -#define arg_error(...) syslog(LOG_ERR, ## __VA_ARGS__); - - for (; options->name; ++options) { - const char *name = options->name; - const char *value = options->value; - - if (strcmp(name, "framerate") == 0) { - int val = atoi(value); - if (val > 0) { - settings.fps = val; - } - else { - arg_error("wrong framerate arg %s\n", value); - } - } - if (strcmp(name, "mjpeg.quality") == 0) { - int val = atoi(value); - if (val > 0) { - settings.quality = val; - } - else { - arg_error("wrong mjpeg.quality arg %s\n", value); - } - } - } + return SPICE_VIDEO_CODEC_TYPE_MJPEG; +} + +Settings &MjpegPlugin::PluginSettings() +{ + return settings; +} + +bool MjpegPlugin::ApplyOption(const Option &o, string &error) +{ + // This plugin only relies on base options + return false; } static bool @@ -211,12 +206,7 @@ mjpeg_plugin_init(Agent* agent) if (agent->Version() != PluginVersion) return false; - std::unique_ptr<MjpegPlugin> plugin(new MjpegPlugin()); - - plugin->ParseOptions(agent->Options()); - - agent->Register(*plugin.release()); - + agent->Register(new MjpegPlugin); return true; } </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="">Frediano</div></div></blockquote></div> </body></html>