-
Notifications
You must be signed in to change notification settings - Fork 112
Update counting rule based on new rule structure #53
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
Conversation
- Transform tables to paragraphs.
- Clear explanation of parameters.
- Clear explanation of event data.
* Proposal for #16 to improve introductino section. * fix typos * Remove mentioning of Key as agreed in WG telco. Co-authored-by: davide cristanelli <davide.cristanelli@videotec.com>
Move keys to data where applicable. Remove query rule and declarative motion detector. Note counting rule hasn't been updated.
- Transform tables to paragraphs. - Clear explanation of parameters. - Clear explanation of event data.
doc/Analytics.xml
Outdated
<listitem> | ||
<para role="param">Segments [tt:Polyline]</para> | ||
<para role="text">One or more polylines used for detection. An object must have passed | ||
through all of them to register as a count. ??? or ??? Passing through any of the |
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.
PLease explain the "???" within lines please.
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 want a discussion on the purpose of having multiple polylines. The original spec says that an object has to pass through all the lines to register as a count. During our telco meetings I was hearing some conflicting use-cases where different polylines would cover different doors and would therefore would be individual counts per line. I just want to make it clear what the definition is.
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 filed this issue for the discussion: #54
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.
Both make sense. Suggest to add a threshold parameter for how many lines an object must cross before it triggers a count (default is one). The assumption would be that there is no need to restrict sequencing order. Be aware that this is an array of polylines, so segments of same polyline do not count as multiple crossings.
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 agree on adding a parameter but I think it should be one or all choice. Having a different number for the threshold than number of segments in the array makes it confusing. So a boolean parameter like PassAllSegments makes sense. I think the default value should be true.
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.
Agree that a threshold is a bit complex. Fine with "PassAll". Not sure whether it should be called PassAllSegments or better PassAllPolylines.
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 agree with both points raised by Hans and Fredrik as I can see the confusion pointed out and think "PassAllPolylines" would be my preferred choice. However if you have a use case of a line created crossing i.e. 3 doors in the camera view and all are opened does this trigger one alarm for each or just one alarm? I am just trying to clarify the count point raised.
doc/Analytics.xml
Outdated
<para role="text">One or more polylines used for detection. An object must have passed | ||
through all of them to register as a count. ??? or ??? Passing through any of the | ||
lines registers as a count. ???</para> | ||
<para>ReportTimeInterval [xs:duration]</para> |
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.
Purpose of ReportTimeInterval unclear: The event is a property event and hence there is no need for repetition. Only reason I can imagine is to reduce the number of events in case of frequent updates. Propose to drop
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.
That's true. I think we should make it a non-property event because I think there are use cases for having a periodical update, for statistical purposes. It's another use case of getting event for every object or every n:th object. Maybe we should have two different events.
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.
Don't get the benefit of a non-property event. With a property event the client always knows the last value. If the client wants to do statistics it should just subscribe and then do whatever it needs to create its statistics.
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.
Sorry see question raised above.
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.
Prefer to keep ReportingTimeInterval as optional parameter
doc/Analytics.xml
Outdated
<para>Optional time interval to report count information, starting from midnight. For | ||
example, if the interval is set to PT2H, reports will be published at 2:00 am, 4:00 | ||
am, etc. Default is same as ResetTimeInterval.</para> | ||
<para>ResetTimeInterval [xs:duration]</para> |
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 about a mechanism to manually reset the counters? ResetInterval should be optional in case someone doesn't want to have automatic reset. Additionally it should be defined what happens in case of overflow: either stop or wrap. Since count type currently is an int it should wrap without going through negative numbers.
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.
Manual reset would require a function call. I don't see a use case for it, and it complicates this feature a lot. We would have to make a new service for this. Regarding overflow, I prefer to stop.
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.
Based on VEWG call on 11/12, below two options were discussed,
1.) one opinion is to update this as ResetTime, where client can specify actual time in xs:time format, since in options we can specify minoccurrs and maxoccurs, client can know how many reset times are possible. eg: can set the reset time as morning 9 am and 6 pm (to calculate day time visitors and night visitors)
2.) Another opinion is to move this out to a separate api.
doc/Analytics.xml
Outdated
<para>ResetTimeInterval [xs:duration]</para> | ||
<para>Periodic count reset time starting from midnight. For example, if the interval | ||
is set to PT12H, the count will be reset at 12:00 noon and 12:00 midnight.</para> | ||
<para>Direction [tt:Direction]</para> |
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.
Our expectation for direction any is that both directions count up. Maybe we need to add two Any-modes
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.
Then the occupancy use-case is not possible. On the last call I thought that was what the majority wanted. Then I propose to use two counters again.
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.
Agree with Hans opinion, when the direction is any count should increment both ways.
<tt:SimpleItemDescription Name="ReportTimeInterval" Type="xs:duration"/> | ||
<tt:SimpleItemDescription Name="ResetTimeInterval" Type="xs:duration"/> | ||
<tt:SimpleItemDescription Name="Direction" Type="tt:Direction"/> | ||
<tt:ElementItemDescription Name="Segments" Type="tt:Polyline"/> |
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 tt:Polyline or tt:PolylineArray ? if its segments polyline array makes sense. Other option is to change this segments to just segment.
How can we notify the number of segments supported, using generic options ?
Instead of segments, from hanwha side we prefer just single line segment.
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.
Prefer to keep Segments as also used in LineDetector. My understanding is that one Polyline consists of a number of segments and hence the name Segments was chosen back in 2008.
# Resolved Conflicts in doc/Analytics.xml
…ename Direction to Mode and move up. Replace ResetTimerInterval by array ResetTime.
doc/Analytics.xml
Outdated
<para role="text">Optional time interval to reduce number of reported changes by reporting aggregated values.</para> | ||
<para role="param">ResetTime - optional, unbounded [xs:time]</para> | ||
<para role="text">Time or times of the day when the counter should be reset. | ||
<emphasis>Question: localtime or utc?</emphasis></para> |
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.
Local time seems to be most commonly used, see e.g. Scheduling service
<varlistentry> | ||
<term>Data</term> | ||
<listitem> | ||
<para role="param">Count [xs:int]</para> |
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.
Change to xs:nonNegativeInteger to emphasize it only counts upwards?
<tt:Data> | ||
<tt:SimpleItemDescription Name="Count" Type="xs:nonNegativeInteger"/> | ||
<tt:SimpleItemDescription Name="Count" Type="xs:int"/> |
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.
Change back to xs:nonNegativeInteger to emphasize it only counts upwards?
doc/Analytics.xml
Outdated
reporting aggregated values.</para> | ||
<para role="param">ResetTime - optional, unbounded [xs:time]</para> | ||
<para role="text">Time or times of the day when the counter should be reset. | ||
<emphasis>Question: localtime or utc?</emphasis></para> |
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.
Local time seems to be most commonly used, see e.g. Scheduling service
doc/Analytics.xml
Outdated
<tt:ElementItemDescription Name="Segments" Type="tt:Polyline"/> | ||
<tt:SimpleItemDescription Name="ReportTimeInterval" Type="xs:duration"/> | ||
<tt:SimpleItemDescription Name="ResetTime" Type="xs:time"/> | ||
<tt:SimpleItemDescription Name="Direction" Type="tt:Direction"/> |
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.
In occupancy counter direction parameter is not required, since its already defined above which direction it will be incremented and decremented. Please check.
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.
Request to update the header for the section A.5 as A.5 Line crossing counting rule as line counting rule is a bit incomplete.
Thanks for spotting the inconsistency. Removed direction from occupancy counter example. |
Rename rule to "line crossing counting" |
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.
Reviewed changes and found no issues. Approve
* Update Analytics.xml Change 'shall' to 'should' in partial config updates requirement * Align message source definition with Annex A update. Insert reference to avoid that the items are defined by examples. * Clarify class likelihood. (#78) * Clarify class likelihood. * Range is [0..1]. * Remove requirement that sum shall be <= 1. * Update example. Change-Id: Ie0574ad87e2d6924c5c335e5b18ba20fb8026a6b * Typo Change-Id: I1adb70934a52a318db3772ee0d9af4648f16ce89 * Use enumerations for AudioClassType restriction values (#75) Closes #72 * Add capability to signal no support for discovery. (#74) * added CertPathValidationPolicyID to EventBrokerConfig (#76) * added CertPathValidationPolicyID to EventBrokerConfig * Annex D: updated to include analytics module and rule based on generic description language (#79) * added common requirement for field parameter for region based detector rules (#81) Co-authored-by: Sriram Bhetanabottla <sriramb@axis.com> * Update counting rule based on new rule structure (#53) * Split the counter into line crossing and occupancy counter. * Clarify reset time as local time. * Remove Direction from example to align OccupancyCounter example with specification. * Move sigC definition to 5.4 since it may apply to any signing not onl… (#83) * Move sigC definition to 5.4 since it may apply to any signing not only repeated. Refactor layout since structure got lost in docbook format. * Remove outdated explanation related to obsolete version zero using UUID references. (#84) * Core/attr list (#87) * Align simple item list types with elements for float and int. Reuse onvif.xsd list types instead of redefining. * Remove accidentally added file devicemgmt.xml. * Core/app mgmt links (#82) * Add two optional links for app configuration. * Add error code for version mismatch. * Clarify that the URI refers to the interface definition not the interface itself. Improve requirements. * Improve section on timeout and keep-alive handling. (#88) * Add reference to RTSP 2.0. Improve section on timeout and keep-alive handling. Improve chapter structure of the RTSP section. * Related to #88: remove deprecated method SetTimeout also from chart. * Replace text with xref in links to 5.2.3 section (#89) Some places with links to 5.2.3 section in Analytic service spec are designed as a text instead of <xref linkend="_Ref9001629" />. * Fix maxOccurrs to maxOccurs typo (#93) Occurrence section has typo: maxOccurrs (double 'r') * Fix order of parameter items in tt:LineCounting (#92) Description of tt:LineCounting has invalid order of items inside Parameters. ElementItemDescription shall follow after all SimpleItemDescription according to xml schema of ItemListDescription type. * Fix incorrect xml, Class can only exist once per object (#99) * Analytics/coordinate system (#94) Replace Default by Normalized to align naming of standard coordinate system. Related to ticket ext-2186. * Add combined type bike. (#95) * Update introduction section to GetEventProperties. (#97) Update introduction section to GetEventProperties. * Fix ABNF missing example changes and improve readability by splitting long construct. (#98) * Fix ABNF missing example changes. * Improve readability by splitting long construct. * Change ABNF code to programlisting. * Replace curved double quotes in ABNF by simple double quotes. * Align example output with analytics spec. * Remove superfluous curly bracket in JSON example Co-authored-by: Fredrik Svensson <fredriks@axis.com> * Sync credential.wsdl to 21.06 branch (#102) This appears to have been lost in transition to github and should have been in 20.12. * coordinates system clarification to media2 (#107) * Add PasswordPolicySupported capability (#105) * Add PasswordPolicySupported capability * capabilities as stringlist * Add flow.svg to new Use case examples appendix (annex) (#96) flow.svg is based on source/flow.uml using http://www.plantuml.com Change-Id: Id42e61714ebc6ead2a69a93b716237701175213c Co-authored-by: Johan Adolfsson <johana@axis.com> * doorcontrol.wsdl: Workaround for xs:any not last in Door (#100) When Door extends DoorInfo it got the extensions in the "middle" of the type which gsoap doesn't like causing occurance violation errors. Instead extend DoorInfoBase and add the Capabilities element first to Door type. This solves https://wush.net/trac/onvif/ticket/2727 Change-Id: Ibb95984a41dfed35fa5cdd6ade5fbb9c45745be0 Co-authored-by: Johan Adolfsson <johana@axis.com> * Fix typo in xml example * Remove hidden annotation with wrong restriction (related to ext-2179) (#110) * Remove hidden annotation with wrong restriction (related to ext-2179) * Fix typo related to change. * Change example coordinates to normative (related to ext-2214) (#109) * Update revision information. * Update revision information. * Update version information. * Update version information. * Update version information. * Update version information. * Update version information. * Update version information. * Update version information. * Update version information. * Update replay.wsdl * Update version information. * Update version information. * Apply review findings: 1. short title "AppMgmtSrv" vs "App Mgmt. Service". Other specs omit service. Should be corrected to "App Mgmt." 2. Missing line breaks in 6.2.1 after http codes 3. Missing explanation alignment of list in section 6.3.1 4. GetCapabilities response has wrong namespace timg (both old & new) 5. Figure 1 a bit too large and not centered because wrong dimensions (pixel instead of mm). Fixed media directory name * Review feedback: replace orderedlist because not supported by stylesheets. * Correction of typos from specification review. * Improve formatting based on review feedback. * Move new item CertPathValidationPolicyID to extension point to avoid breaking change. (#117) Co-authored-by: OksanaTyushkina <76164127+OksanaTyushkina@users.noreply.github.com> Co-authored-by: Fredrik Svensson <fredriks@axis.com> Co-authored-by: Matthew Brush <mbrush@codebrainz.ca> Co-authored-by: Davide Cristanelli <davide.cristanelli@videotec.com> Co-authored-by: sujith raman <67031640+sujithhanwha@users.noreply.github.com> Co-authored-by: Sriram Bhetanabottla <1432443+bsriramprasad@users.noreply.github.com> Co-authored-by: Sriram Bhetanabottla <sriramb@axis.com> Co-authored-by: johado <papputten@gmail.com> Co-authored-by: Johan Adolfsson <johana@axis.com>