GNOME Bugzilla – Bug 588252
Set the codecs preferences/properties from a config file
Last modified: 2009-08-07 11:44:36 UTC
Added two patches to set the codec preferences/properties from a config file I'm putting in /usr/share/empathy (not /etc) because its more a developer feature than a sysadmin feature).
Created attachment 138196 [details] [review] Put codec preferences in a configuration file
Created attachment 138197 [details] [review] Make it possible to set element properties from a config file
Wouldn't it be better to have this in gconf? This would for example give lockdown for free.
Having a GKeyFile in GConf ? Also, Vuntz loves you
Do we really want to let the user define its own file for that? I think out of the 6billion people on earth, only Olivier knows what it is about... Can't we just ship the file in /usr/share/empathy and that's it?
Created attachment 138218 [details] [review] Put codec preferences in a configuration file Used empathy_file_lookup() to find the file, and I removed the lookup from the user dir, I was mostly convinced by Xavier's argument that only experts know which codecs should be preferred (and distros can use it to disable broken codecs)
Created attachment 138219 [details] [review] Make it possible to set element properties from a config file
Okay, so it's not about configuration or preferences, just a data file used by Empathy; fine.
Created attachment 138344 [details] [review] Added element properties to ensure h263 packets are small enough (MSN)
I'm not a big fan of forcing a-mode on everyone.. but I'm not sure what the best way to have per-protocol properties is... Well, ideally it would be in the SDP, but its not there :-(
(In reply to comment #10) > I'm not a big fan of forcing a-mode on everyone.. but I'm not sure what the > best way to have per-protocol properties is... Well, ideally it would be in the > SDP, but its not there :-( Is that different from other codec options? Will that cause troubles with other protocols?
Mode A only fragments at GOB boundaries, so it may not be able to fragment large images enough forcing the fragmentation to be done at the IP level (for ipv4) or completely preventing it from working (if using ipv6)...
Should we add a way to specify element properties in tp-spec ? Or pass that information along the codecs (SetRemoteCodecs) in the parameters ?
Created attachment 138409 [details] [review] Put codec preferences in a configuration file
Created attachment 138410 [details] [review] Make it possible to set element properties from a config file
1. Update the codec prefs to use "clock-rate" instead of "rate" .. oops 2. Updated the element properties patch to set the ffenc_h263 rtp-payload-size prop. 3. The mode-a-only will go into the SDP caps
Comment on attachment 138410 [details] [review] Make it possible to set element properties from a config file >- g_signal_connect (priv->fsnotifier, "element-added", >- G_CALLBACK (conference_element_added), NULL); >+ keyfile = g_key_file_new (); >+ filename = empathy_file_lookup ("element-properties", "data"); >+ if (g_key_file_load_from_file (keyfile, filename, G_KEY_FILE_NONE, NULL)) >+ fs_element_added_notifier_set_properties_from_keyfile (priv->fsnotifier, >+ keyfile); >+ else >+ g_key_file_free (keyfile); >+ g_free (filename); Doesn't the coding style mandate braces? Also, g_key_file_load_from_file should have its error handled. Also, what is freeing the keyfile when g_key_file_load_from_file returns TRUE?
(In reply to comment #17) > Doesn't the coding style mandate braces? Which coding style is it even supposed to be? > Also, g_key_file_load_from_file should have its error handled. Yes, it should print a g_warning() I guess > Also, what is freeing the keyfile when g_key_file_load_from_file returns TRUE? FsElementAddedNotifier takes ownership of it.
Created attachment 138641 [details] [review] Put codec preferences in a configuration file Now prints a g_warning if the config file can't be found
Created attachment 138642 [details] [review] Make it possible to set element properties from a config file Also prints a g_warning if the properties file can't be found
Created attachment 139659 [details] [review] Correctly register the src/sink elements This allows the next patch to work
Created attachment 139660 [details] [review] Move the FsElementElementAddedNotifier over the main pipeline With this patch, we can use the element-properties to set properties on alsasrc and the like to control the latency and stuff
Created attachment 139895 [details] [review] Force skew method of clock slaving for alsa/osssrc Without this patch, the timestamps will be wrong and the audio will be mangled at the receiving side
Created attachment 139986 [details] [review] Correctly register the src/sink elements Updated to do the "do once" thing correctly... Available as a branch at: http://git.collabora.co.uk/?p=user/tester/empathy.git;a=shortlog;h=refs/heads/calling-fixes
Ok, please merge :)