Error during startup on Windows for missing pulse plugin

[farazs]

Occurs after f18760bf19d46845feb7c624a47615b4bc617b55 committed from PR #654.

Either my computer, or the Windows version doesn't have the pulse plugin. Normally, this was handled fine by not including it in the list of possible audio inputs. Getting this error on startup now. I think it's because to initialize the config, the plugin itself is being instantiated.

2014-11-17 16:54:06,838 ( ERROR) yapsy : Unable to import plugin: C:\Users\faraz\Documents\cs\cs499\Freeseer\src\freeseer\plugins\audioinput\pulsesrc Traceback (most recent call last): File "C:\Program Files (x86)\Python27\lib\site-packages\yapsy\PluginManager.py", line 484, in loadPlugins candidate_module = imp.load_module(plugin_module_name,None,candidate_filepat h,("py","r",imp.PKG_DIRECTORY)) File "C:\Users\faraz\Documents\cs\cs499\Freeseer\src\freeseer\plugins\audioinput\pulsesrc\__init__.py", line 72, in <module> class PulseSrcConfig(Config): File "C:\Users\faraz\Documents\cs\cs499\Freeseer\src\freeseer\plugins\audioinput\pulsesrc\__init__.py", line 74, in PulseSrcConfig source = options.StringOption(get_default_source()) File "C:\Users\faraz\Documents\cs\cs499\Freeseer\src\freeseer\plugins\audioinput\pulsesrc\__init__.py", line 65, in get_default_source sources = get_sources() File "C:\Users\faraz\Documents\cs\cs499\Freeseer\src\freeseer\plugins\audioinput\pulsesrc\__init__.py", line 56, in get_sources audiosrc = gst.element_factory_make("pulsesrc", "audiosrc") ElementNotFoundError: pulsesrc

[farazs]

It might be hard for Francisco to fix if he's not running Windows, so I might give it a try.

[zxiiro]

This plugin shouldn't even be attempted to load on Windows (https://github.com/Freeseer/freeseer/blob/f18760bf19d46845feb7c624a47615b4bc617b55/src/freeseer/plugins/audioinput/pulsesrc/__init__.py#L79).

If the code is no longer checking for supported OSes before loading a plugin then this is a regression.

[dideler]

Thanks for reporting this @farazs. I overlooked multi-platform support when merging. If a fix is coming soon then I don't think I'll revert f18760b because then "USB source" recording will break in Linux and Linux support is generally higher priority.

[farazs]

So, a simple solution to this might be to define the os list in PulseSrcConfig as well and only initialize the source variable if sys.platform is in os.

But I'm wondering, is there a reason why these variables are being initialized as class variables when there is no instance of the class? For example, could we just initialize them once in an __init__() method instead and initialize them to '' in the class definition, or even not define them at all outside of init? It seems an instance is created in get_config() in profile.py before it is passed to storage.load().

In fact, is there a reason we need them to be class variables and not instance variables? Could we just keep them as instance variables? Or is that the way Freeseer config classes and the config parsers work?

[zxiiro]

This was coded for yapsy and with yapsy it is the way you retrieve this kind of variable easily without writing your own plugin loader. If that's changed then we need to fix the code. This worked just fine when we were using yapsy. I heard rumors of someone rewriting our plugin system since and have not looked at the new code in detail so I'm not familiar with the current plugin loader if it's changed.

[zxiiro]

Also it is possible to load the same plugin twice. For example loading 2 usb sources to use picture in picture.

[farazs]

If the same plugin was loaded twice, and the details about the plugin configuration are being stored as class variables, wouldn't this cause issues? (ie. selecting a different device for one USB plugin would overwrite the device class variable and this would affect both plugins)

They were also stored as class variables before the plugin configuration refactoring though so if it worked then it should work now.

[farazs]

I believe the plugins are now using ConfigParserStorage or JSONConfigStorage to store their configuration details. I'm not sure how multiple instances of the same plugin are being handled.

@FranciscoCanas Do you know if loading the same plugin twice case is handled? I don't know much about the Freeseer config storage classes.

I think if the variables are initialized to '' in the class definition and then set to some meaningful defaults only on the first __init__ call that might work. But I'm not sure if #654 might come back.

I'll try creating a pull request with this and see if @dideler can confirm that #654 doesn't come back.

Does anyone have 2 usb sources they can use to see if that's working fine?

[zxiiro]

It's a class variable not a global variable. In other languages these variables should actually be static final variables. It shouldn't change and the OS values are meant to be read only anyway. There shouldn't be any code to change these values.

[zxiiro]

For testing you could also use 2 test video sources, just make one of them bigger than the other.

[farazs]

Hmm so I tried it with two desktop sources and I can't set different regions for them. The settings for one affect the settings for the other. But I went back to the first commit of the term and it doesn't work then either. Did it work at some point in the past?

[zxiiro]

I'm pretty sure it worked when we were on yapsy code over a year ago. I used my built in laptop webcam + an external webcam to add that feature. But my original code did not support changing configuration and I hardcoded it in my proof of concept. I thought a student either last term or the term before implemented the feature to configure settings via plugin instances. This was why freeseer had the plugin-0, plugin-1, etc... For configuration sections in plugin.conf. It was to differentiate the plugin instanced configuration.

[FranciscoCanas]

@farazs Multiple versions of the same plugin are handled by writing their Configs to file under different IDs: so like USB Source-0, USB Source-1, etc. The IBackendPlugin class has a method called set_instance() that sets this ID, and this method is called when loading plugin info into widgets and such. If I understood correctly, the reason it was done this way originally was because Yapsy PluginManager treats plugins as singletons.

For the current issue where pulsesrc is loaded in Windows: I see that the get_default_source() method I added in #654 is being run when freeseer's PluginManager initializes at start-up, but inside the collectPlugins() method of the Yapsy PluginManager. So I agree with @farazs the better solution is to change the default values back to '', then call the get_default_source() when the plugin is loaded through the widgets.

I can look into this some more tomorrow, and will update when I find out more.

[FranciscoCanas]

I've opened #660 with a proposed solution:

It moves the calls to get_default_device() into the get_audio/videoinput_bin() methods, so get_default_device should only run if the plugin is actually being used, and not during initialization of the PluginManager.

I can't verify if this fixes the windows issue, but I did step through the program to make sure the code causing the exception is no longer run on start up (it is only run if I select pulsesrc as my audio plugin and then click the 'prepare to record' button).

@farazs Would you be able to check this branch out and test it on windows?