GNOME Bugzilla – Bug 733233
chromaprint: notify fingerprint also via property notify
Last modified: 2016-04-07 19:57:15 UTC
Created attachment 280773 [details] [review] proposed solution I think, chromaprint plugin should notify user about fingerprint not by Tag, but by property change notification. I implemented notify-based solution, and left tag-method too, for backward compatibility.
Comment on attachment 280773 [details] [review] proposed solution Please get rid of all the unrelated whitespace changes. And I think for consistency it would be good to just store all pspecs in an array indexed by the property enum
I would just emit the notify by name, the pspec thing is only really needed if this is something you might be doing a *lot* and performance is important. Not sure that is the case here. The reason we tend to not notify such things via properties is that the notification will come from a streaming thread, which causes all kinds of problems.
Created attachment 280895 [details] [review] proposed solution v2 Thanks for your advices, I improved my patch a little.
Thanks, pushed (I removed the line break in the property registration and your copyright addition - we usually don't add copyright notices for minor additions): commit ac8a14d1c825d572fc901490f7658f1a08ff68fe Author: Marcin Kolny <marcin.kolny@gmail.com> Date: Wed Jul 16 02:44:42 2014 +0200 chromaprint: emit notify::fingerprint signal when fingerprint is ready In addition to adding the fingerprint to the tags. https://bugzilla.gnome.org/show_bug.cgi?id=733233