GNOME Bugzilla – Bug 771261
opusenc: review misleading "audio-type" property, possibly add "signal-type" property
Last modified: 2018-11-03 11:49:54 UTC
Created attachment 335319 [details] [review] patch that adds new property signal-type to opusenc Currently, opusenc has no setting that can be used to call OPUS_SET_SIGNAL ctl. The attached patch adds new property signal-type to opusenc.
I wonder if we should do all of this (including bug #771262 and bug #771263) a bit differently. Maybe merge those bugs into one so we can deal with all of it in one go. The current property is a bit weird indeed, not sure how that slipped though :) And we should not have exposed the opus values directly, but made our own enum and then mapped that to the opus values. So I wonder if what we should do is: - change what "audio-type" does under the hood to OPUS_SET_SIGNAL - add new "sane" enum values for "audio-type" and deprecate the existing ones - add a new "application-type" property for the latency/coding trade-offs - if "audio-type" is set to the deprecated voice type we default to the appropriate voip application-type if application-type hasn't been set explicitly already. How does that sound?
Generally speaking I agree with your proposal to make this done right. Except: 1. I think audio-type should remain as it is, it definitely should not silently change behaviour, just should be marked as deprecated and removed in the future. I imagine all these angry developers that figure out that their app started to work differently after upgrade of GStreamer. Then we can add signal-type, that matches Opus naming (which IMO is itself a plus), that uses OPUS_SET_SIGNAL under the hood. 2. I think we should not touch enums of existing properties, I've seen apps in the wild that use "insane" values that are taken straight from Opus. Such change will break them. I initially didn't wanted to change the API of opusenc, but if you agree to do so I can prepare the patch that merges all these fixes. Please confirm :)
My proposal would not break anything for existing applications. We would leave in place the existing enums for backwards compatibility. I think "signal-type" is an awful name (from an end-user point of view), and while I agree that it's generally a good idea to keep things in sync with the codec API where possible, I'm not convinced this is one of those times.
*** Bug 771263 has been marked as a duplicate of this bug. ***
*** Bug 771262 has been marked as a duplicate of this bug. ***
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-base/issues/292.