use-go / onvif

full and enhanced onvif protocol stack in golang.
MIT License
384 stars 180 forks source link

Media structs are prefixed with onvif: #29

Open wfotiuk opened 1 year ago

wfotiuk commented 1 year ago

So I noticed when trying to use the media queries (sdk/media/*), that some of the structs are prefixed by onvif: for the xml parsing tags (example https://github.com/use-go/onvif/blob/master/xsd/onvif/onvif.go#L361).

image

When getting an actual response from a camera, I noticed the structs did not unmarshal properly because of this prefix. From my understanding, the prefixes should be different according to the ONVIF media spec.

image

Regardless, I have forked the repo and dropped the prefix for my uses, but was curious if you would like a PR and the tests I wrote, or if I am headed in the wrong direction

wfotiuk commented 1 year ago

After investigating further, I think the issue is that the xml spec defined in Device.go uses onvif in place of tt as a prefix. This causes actual responses from cameras (which use tt according to ONVIF standards, which are followed escept for the onvif prefex). I am happy to fix this and add some tests if you would like

guo1017138 commented 1 year ago

I have the same issue. My camera returns response with tt prefix and the response struct in Golang can't get any value which key has prefix with tt. image

wfotiuk commented 1 year ago

Hi @guo1017138, so we ended up forking the repo and I can give you some insights as to what is happening. There is two issues:

  1. The ./Device.go file contains a map of XML prefixes var Xlmns = map[string]string{.... In this map, instead of http://www.onvif.org/ver10/schema being mapped to tt it is mapped to onvif. Changing this helps things but still leaves a problem.
  2. The way go handles xml marshaling and unmarshaling is kind of strange. I found that we had to make different response and request structs, because when marshaling into xml it needed the tt prefix on struct tags (eg .xml:"tt:Type"), but when unmarshaling into a go struct it needed to NOT have the tt tags in order to work (eg xml:"Type"). This is due to a long standing bug in go xml parsing that they tried to fix many years ago but ended up breaking other things so they reverted it.

I hope that helps you!

guo1017138 commented 1 year ago

@wfotiuk Thanks for the nice suggestion/solution. My private change solution is almost the same with you, especially for item 2. I will apply item 1 also to make it more perfect!

wfotiuk commented 1 year ago

Right on! I am going to be looking into putting up some PRs getting this stuff into the main repo if possible.

tarancss commented 1 year ago

I faced similar problems and also ended up forking the repo. I opened #27 some time ago with some other general fixes.

Pawan-ky commented 10 months ago

Right on! I am going to be looking into putting up some PRs getting this stuff into the main repo if possible.

@wfotiuk

would love to see you changes, if you have some already can you please share that

wfotiuk commented 10 months ago

I can! I have created my own fork of it and I have to talk to my boss to see what we can share. I'll get back to you!

On Sun, Aug 27, 2023 at 10:48 PM Pawan-ky @.***> wrote:

Right on! I am going to be looking into putting up some PRs getting this stuff into the main repo if possible.

@wfotiuk https://github.com/wfotiuk

would love to see you changes, if you have some already can you please share that

— Reply to this email directly, view it on GitHub https://github.com/use-go/onvif/issues/29#issuecomment-1695049572, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYXHQWPUEQ42ETQYSTLMK3XXQWJZANCNFSM6AAAAAARDNNQGI . You are receiving this because you were mentioned.Message ID: @.***>

-- -Wally Fotiuk

wfotiuk commented 10 months ago

I can share everything I think but I have altered the repo quite a bit. Maybe we can have a meeting to go over the changes if you have time. I added generated tests for the generated functions and had to change some structures because they would serialize fine but break when deserializing. I can compile a list of the changes and we can incorporate what we want to go into the main trunk. I do think all of the changes were necessary to make the repo functional

On Mon, Aug 28, 2023 at 11:16 AM Wally Fotiuk @.***> wrote:

I can! I have created my own fork of it and I have to talk to my boss to see what we can share. I'll get back to you!

On Sun, Aug 27, 2023 at 10:48 PM Pawan-ky @.***> wrote:

Right on! I am going to be looking into putting up some PRs getting this stuff into the main repo if possible.

@wfotiuk https://github.com/wfotiuk

would love to see you changes, if you have some already can you please share that

— Reply to this email directly, view it on GitHub https://github.com/use-go/onvif/issues/29#issuecomment-1695049572, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYXHQWPUEQ42ETQYSTLMK3XXQWJZANCNFSM6AAAAAARDNNQGI . You are receiving this because you were mentioned.Message ID: @.***>

-- -Wally Fotiuk

-- -Wally Fotiuk