Skip to content
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

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Conversation

sujithhanwha
Copy link
Contributor

Audio EQ Preset related change

AudioOutputConfigurationOptions updated
SetEQPreset Method added
AudioOutputConfiguration updated with EqPreset token.

<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">
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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.

</xs:element>
<xs:element name="Gain" type="xs:float">
<xs:annotation>
<xs:documentation>Normalized value (0.0 ~ 1.0).</xs:documentation>
Copy link

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?

Copy link
Contributor Author

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.

<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>
Copy link

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.

Copy link
Contributor Author

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.

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

<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">
Copy link

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?

Copy link
Contributor Author

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">
Copy link

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?

Copy link
Contributor Author

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.

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>
Copy link

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?

Copy link
Contributor Author

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

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">

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?

Copy link
Contributor Author

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">
Copy link
Contributor

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?

Copy link
Contributor Author

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>
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants