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

Improve getAudioEncoderConfigurationOptions. #150

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bl0ggy
Copy link
Contributor

@bl0ggy bl0ggy commented Feb 21, 2020

Permit to get an AudioEncoderConfigurationOption with either a ConfigurationToken, a ProfileToken or both as first argument.
Update JSDoc.

This breaks backward compatibility. But I wonder, I updated the response XML file because with the cameras I have (Samsung and HIK Vision), it's very different. First, there are tt:Options tags inside the trt:Options tag, second, the elements in lists are not tt:List but tt:Items. Did the structure change over years ? Are there cameras still responding the old way ? If that's the case, we will have to convert one structure into the other.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 87.929% when pulling f89f992 on bl0ggy:improve_getAudioEncoderConfigurationOptions into f8ee914 on agsh:master.

@RogerHardiman
Copy link
Collaborator

Did the structure change over the years.....

Yes the structure did change.
Originally we had the Media Service
https://www.onvif.org/ver10/media/wsdl/media.wsdl

Now we also have Media2 (Media Service Version 2.0 and the URL has ver20 in it)
https://www.onvif.org/ver20/media/wsdl/media.wsdl

So we need to check if a camera uses Media1 or Media2 and then parse the XML differently depending on which Media Service (media 1 or media 2) the camera supports.

@RogerHardiman
Copy link
Collaborator

Just to add some extra info.
In the original ONVIF spec we had to get the System Date and Time and then call GetCapabilities to get the URLs for each 'Service' (media, imaging, PTZ, device etc)
The ONVIF Library's constructor does this as shown below (from cam.js)

/**

  • Connect to the camera and fill device information properties

  • @param {Cam~ConnectionCallback} callback
    */
    Cam.prototype.connect = function(callback) {

     // Must execute getSystemDataAndTime (and wait for callback)
    
     this.getSystemDateAndTime(function(err, date, xml) {
             if (err) {
                     return callback.call(this, err, null, xml);
             }
             this.getCapabilities(function(err, data, xml) {
    

If you read the latest ONVIF Core Specification it says we are supposed to call getSystemDateAndTime and then call GetServices

The old GetCapabilities ONVIF command is now deprecated and left in for backwards compatibility.

8.1.2.4 GetCapabilities
This method provides a backward compatible interface for the base capabilities. Refer to
GetServices for a full set of capabilities.

So actually there is a larger issue here.
What we need the ONVIF library to do is
a) getSystemDateAndTime
b) try GetServices. If it works we will know if we are using Media1 or Media2
c) if GetServices fails, we need to fall back to GetCapabilities

So before we can implement this PR we need to fix some other things in the library.

Let me know if you need any more info.

Roger

@RogerHardiman
Copy link
Collaborator

Hi.
Just a reminder you need to modify the commit to handle 'media' and 'media2' as the APIs are different.

@bl0ggy
Copy link
Contributor Author

bl0ggy commented Mar 17, 2020

I have a few questions.

  • Should I discard the changes I made to the xml response ? Because media v1 does not have tt:List anymore
  • Should I make a getAudioEncoderConfigurationOptionsV2 function ?
  • The xmlns in the request must be modified from ver10 to ver20 too, how to know which version to use ?
  • I can't find where I read it but I remember that media is for Profile S while media2 is for Profile T, is that correct ?

@RogerHardiman
Copy link
Collaborator

Old ONVIF used the GetCapabilities command to get the ONVIF Services like device and media.

New ONVIF uses the GetServices command to get the Device Services like Media2.

So first step is to edit the library when it first connects to the camera.
You need to try Get Services first and fail back to GetCapabilities.
If GetServices works you can check if it reports Media or Media2 support.

You need to set a global flag for the class and use it to decide what format XML to send.

Roger

@RogerHardiman
Copy link
Collaborator

Hi @bl0ggy . Do you still have access to your Samsung and Hik cameras?

@RogerHardiman
Copy link
Collaborator

Hi Jessy, I've made use of your changes to detect Media2 in my 'roger' branch on github.
You can now use this to create a Media2 specific version of getAutioEncoderConfigurationOptions that handles the different XML format.

For getProfiles, getStreamUri and getSnapshotUri I made the function handle media1 and media2 and I modified the result so Media2 results look like Media1 results.
But your suggestion of having functionNameV2() with V2 added is a good idea.

Roger

@bl0ggy
Copy link
Contributor Author

bl0ggy commented Apr 11, 2021

Hi @RogerHardiman, yes I still have access to some Samsung cameras and at least one Hik.
Unfortunately I don't have much time for that, but I have a coworker who may be able to do it.
We will keep you informed.

@RogerHardiman
Copy link
Collaborator

@bl0ggy . Just wanted to let you know that in the summer I added code to detect Media API and Media2 API for video.
So we could re-visit your code for parsing the Audio Sources

@agsh
Copy link
Owner

agsh commented Feb 15, 2022

@RogerHardiman I saw your improvements and tried to copy them to the typescript version. So, what to do you think now?

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

Successfully merging this pull request may close these issues.

4 participants