-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Audio EQPreset Change #450
base: development
Are you sure you want to change the base?
Conversation
wsdl/ver10/schema/onvif.xsd
Outdated
<xs:any namespace="##any" processContents="lax" minOccurs="0" maxOccurs="unbounded"/> <!-- first Vendor then ONVIF --> | ||
</xs:sequence> | ||
<xs:anyAttribute processContents="lax"/> | ||
</xs:extension> | ||
</xs:complexContent> | ||
</xs:complexType> | ||
|
||
<xs:simpleType name="FrequencyRange"> | ||
<xs:restriction base="xs:string"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this frequency range list standard for all type of EQPresets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this frequency range list standard for all type of EQPresets?
This is the common frequency range list, it can be different also. This enumeration is only for reference, the actual data type for this frequency range is xs:string, therefore we can extend it in future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the xs:string for further extensibility.
wsdl/ver10/schema/onvif.xsd
Outdated
</xs:element> | ||
<xs:element name="Gain" type="xs:float"> | ||
<xs:annotation> | ||
<xs:documentation>Normalized value (0.0 ~ 1.0).</xs:documentation> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question on this scale, what do this normalized value mean? Is it linear, logarithmic? Only attenuation, not amplifiation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated this to decibel based on Bangkok f2f discussion. Please re-check the updated commit.
wsdl/ver10/schema/onvif.xsd
Outdated
<xs:sequence> | ||
<xs:element name="FrequencyRange" type="xs:string"> | ||
<xs:annotation> | ||
<xs:documentation>Defined in tt:FrequencyRange, e.g., 20Hz-60Hz, 2kHz-4kHz.(Readonly)</xs:documentation> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessasary to pre-define list or more dynamically let device return capabilites with ranges and gain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Device is free to choose its own, this is only a guideline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the FrequencyRange enum is only for guideline, I suggest dropping the pre-defined from the spec to avoid confusion. Instead, add more documentation on what to expect if necessary. Also, a common guideline for EQ is hard, right? As it exists many different implementations for different products and use cases. For example, another guideline would be a 10-band EQ based on the octave band (as described here https://en.wikipedia.org/wiki/Octave_band, also coming from an IEC standard).
I also suggest moving from specifying a frequency range to instead set the center frequency of the band. This would increase possibility to compare between products as the range may differ depending on kind of filter and bandwidth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the FrequencyRange enum is only for guideline, I suggest dropping the pre-defined from the spec to avoid confusion. Instead, add more documentation on what to expect if necessary. Also, a common guideline for EQ is hard, right? As it exists many different implementations for different products and use cases. For example, another guideline would be a 10-band EQ based on the octave band (as described here https://en.wikipedia.org/wiki/Octave_band, also coming from an IEC standard).
I also suggest moving from specifying a frequency range to instead set the center frequency of the band. This would increase possibility to compare between products as the range may differ depending on kind of filter and bandwidth.
@robberos , agree with your suggestion will discuss this change in upcoming telco and will update it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sujithhanwha @robberos I also found your suggestion interesting after reviewing your link.
wsdl/ver10/schema/onvif.xsd
Outdated
<xs:documentation>Indicates whether the device allows changing the gain level for frequencies. (Readonly)</xs:documentation> | ||
</xs:annotation> | ||
</xs:element> | ||
<xs:element name="FrequencyGainPair" type="tt:FrequencyGainPair" minOccurs="0" maxOccurs="unbounded"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for one EQPreset, is it required to return a specific frequency list or skip it to not show specific implemantation details?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes its implementation specific.
<!--===============================--> | ||
<!-- EQPreset --> | ||
<!--===============================--> | ||
<xs:complexType name="EQPreset"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EQPreset seems as a combined capabilities and configuration structure. Split into seperate?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original proposal was something similar to have separate method, based on feedback from previous meetings, its defined like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks. resolved 👍
</xs:element> | ||
<xs:element name="ScheduleToken" type="xs:string" minOccurs="0"> | ||
<xs:annotation> | ||
<xs:documentation>Optional schedule token (if supported).</xs:documentation> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is schedule to be used? Able to set many presets?
See also <xs:element name="Configuration" type="tt:EQPreset"> in SetEQPresetConfiguration, to be a list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now this parameter is removed in latest commit and clarified the issue raised during last F2F meeting in Bangkok.
Please check the latest commit0059eac
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, EQPresets now unbounded. resolved
@@ -2134,6 +2158,15 @@ image will be updated automatically and independent from calls to GetSnapshotUri | |||
<soap:body parts="parameters" use="literal"/> | |||
</wsdl:output> | |||
</wsdl:operation> | |||
<wsdl:operation name="SetEQPreset"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we have a GetEQPreset? Even thoug it is a capability what is possible, if setting a EQ for the editable ones, I guess that won't show up in capabilities and instead need to be retrieved via a get-request?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we have a GetEQPreset? Even thoug it is a capability what is possible, if setting a EQ for the editable ones, I guess that won't show up in capabilities and instead need to be retrieved via a get-request?
@robberos, Initially, we planned to include geteqpreset. However, during our previous discussions, some members preferred not to add more methods. Instead, we now provide the list of eqpreset in the GetAudioOutputConfigurationOptions response. If you believe this method is necessary, we can discuss it in the next telco.
@@ -1850,6 +1912,16 @@ IN NO EVENT WILL THE CORPORATION OR ITS MEMBERS OR THEIR AFFILIATES BE LIABLE FO | |||
<xs:documentation>Minimum and maximum level range supported for this Output.</xs:documentation> | |||
</xs:annotation> | |||
</xs:element> | |||
<xs:element name="EQPresetScheduleSupport" type="xs:boolean" minOccurs="0"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it is true does it meant that Schedule Service shall be supported by a DUT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, when EQPresetScheduleSupport is true, the schedule service is supported by the DUT and is also available in the EQPreset feature.
</xs:element> | ||
<xs:element name="isDefault" type="xs:boolean"> | ||
<xs:annotation> | ||
<xs:documentation>This shows if the EQ preset is set as the default. When the scheduler isn't active, this preset will be in use. Only one preset can be marked as default among the eqpresets linked to an audio output.</xs:documentation> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I correctly understand that SetEQPreset with isDefault = true sets this preset default for all audio output listed in OutputTokensAvailable of the corresponding option?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your understanding is correct, when its set to default, its applicable for all audio output tokens(OutputTokensAvailable) listed in corresponding option.
Audio EQ Preset related change
AudioOutputConfigurationOptions updated
SetEQPreset Method added
AudioOutputConfiguration updated with EqPreset token.