[Spice-devel] [RFC spice-streaming-agent 1/2] gst-plugin: allow the instantiation of multiple GST encoder plugins

[Christophe de Dinechin]

Kevin Pouget writes:

> On Wed, Jul 31, 2019 at 11:59 AM Christophe de Dinechin
> <dinechin at redhat.com> wrote:
>>
>> I don't really like the way option parsing is implemented, because I
>> think it leads to somewhat counter-intuitive user behaviors.
>>
>> For example, if I say:
>>
>>     gst.codec=mjpeg gst.codec=h264 framerate=20
>>
>> I get two plugins with framerate=20, but if I have
>>
>>     gst.codec=mjpeg framerate=20 gst.codec=h264
>>
>> then the mjpeg plugin has framerate 20 and the h264 plugin has default
>> framerate (25). If I specify
>>
>>     framerate=20 gst.codec=mjpeg gst.codec=h264
>>
>> then I get two plugins with default frame rate.
>>
>> Note that all these behaviors are especially problematic because they
>> would be different from what you get with another plugin (where
>> ParseOption will process all options).
>
> I double checked, but it doesn't run like that, the order is not
> relevant,

Indeed, I mis-read the call to ParseOptions, I thought you were passing
options and not agent->Options().

>
>> > +
>> > +            auto plugin = std::make_shared<GstreamerPlugin>();
>> > +            plugin->ParseOptions(agent->Options(), codec_type);
>> > +            agent->Register(plugin);
>> > +        }
>> > +    }
>>
>> Because of this second loop, you presently loop multiple times over the
>> same options.

Well, I guess if we accept that for different plugins, it's OK to accept
it for multiple instances of the same plugin.

>> >
>> > -    agent->Register(plugin);
>> > +    // no default value at the moment
>> > +    // (used to be h264, see GstreamerEncoderSettings::codec)
>> >
>> >      return true;
>> >  }
>>
>>
>> --
>> Cheers,
>> Christophe de Dinechin (IRC c3d)
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/spice-devel


--
Cheers,
Christophe de Dinechin (IRC c3d)