GNOME Bugzilla – Bug 754819
opusenc: critical/warning when dumping dots
Last modified: 2015-09-11 21:33:35 UTC
Hello, I have noticed that opusenc breaks (eg: when testing) if dumping dots (setting GST_DEBUG_DUMP_DOT_DIR env var). This is caused because when dumping dots all properties are got and for deprecated ones g_warning is used. Example: Unexpected critical/warning: cbr property is deprecated; use bitrate-type instead Source ref: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/ext/opus/gstopusenc.c?id=1.5.90#n1038
Same happens with gst-inspect of course, not sure what to do about that. Not printing the deprecated properties seems not ideal either.
We could check if G_PARAM_DEPRECATED flag is set after getting the property. @sebastian, what do you think about this solution? opusenc uses this flag: http://cgit.freedesktop.org/gstreamer/gst-plugins-bad/tree/ext/opus/gstopusenc.c?id=1.5.90#n316 Check the flag in debug_dump_get_element_params (the same for gst-inspect) http://cgit.freedesktop.org/gstreamer/gstreamer/tree/gst/gstdebugutils.c?id=1.5.90#n117
And then do what based on that flag? Ignoring the property is not a good idea as it might contain useful information if still used by the application...
Other plugins like rtpbin just notify that property is deprecated in the description, for example: use-pipeline-clock : Use the pipeline running-time to set the NTP time in the RTCP SR messages (DEPRECATED: Use ntp-time-source property) flags: legible, escribible, deprecated Boolean. Default: false Adding a critical warning may make test fail, for example generating a dot as Miguel says.
Taking into account all comments, I think that a good solution would be using only g_warning for set_property and not for get_property. Why? From my point of view, the warning makes sense only when the property is set by a user, because he wants deliberately modify the behaviour of the element. A user that only reads the property, does not want to change de behaviour, only see the current value. In the case of plugins description (related with reading properties), I agree with Jose, and a "deprecated" comment should be shown. Maybe, it is not an ideal solution but is better that the current one.
Oh I thought the warning comes from GLib. Yes, only do a g_warning() when setting the property. There are more properties like this in opusenc IIRC. Want to provide a patch?
There are 2 properties: - constrained-vbr - cbr I am attaching a patch ;)
Created attachment 311121 [details] [review] opusenc: do not throw g_warning when getting deprecated properties
Created attachment 311122 [details] [review] opusenc: improve deprecated properties doc
commit 25f0787c1c09757799f7f3dcc6a8cf09d69b9298 Author: Miguel París Díaz <mparisdiaz@gmail.com> Date: Fri Sep 11 11:22:35 2015 +0200 opusenc: improve deprecated properties docs https://bugzilla.gnome.org/show_bug.cgi?id=754819 commit f000cd4963b810b66aa7b8969b40fcf697a05259 Author: Miguel París Díaz <mparisdiaz@gmail.com> Date: Fri Sep 11 11:11:09 2015 +0200 opusenc: do not throw g_warning when getting deprecated properties https://bugzilla.gnome.org/show_bug.cgi?id=754819