8000 Update counting rule based on new rule structure by svefredrik · Pull Request #53 · onvif/specs · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Merged
merged 11 commits into from
Mar 11, 2021

Conversation

svefredrik
Copy link
Contributor
  • Transform tables to paragraphs.
  • Clear explanation of parameters.
  • Clear explanation of event data.

HansBusch and others added 4 commits September 18, 2020 14:17
* 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.
@svefredrik svefredrik added Analytics hacktoberfest-accepted Hacktoberfest contribution labels Oct 30, 2020
@svefredrik svefredrik added this to the video milestone Oct 30, 2020
<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
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author
@svefredrik svefredrik Nov 2, 2020

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

Copy link
Member

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.

Copy link
Contributor Author
@svefredrik svefredrik Nov 11, 2020

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.

Copy link
Member

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.

Copy link
Contributor

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.

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

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

Copy link
Contributor Author
@svefredrik svefredrik Nov 11, 2020

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.

Copy link
Member

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.

Copy link
Contributor

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.

Copy link
Contributor

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

<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>
Copy link
Member
@HansBusch HansBusch Nov 10, 2020

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

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

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

Copy link
Contributor Author

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.

Copy link
Contributor

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

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.

Copy link
Member

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.

Base automatically changed from video/annex-a-rules to video/annex-a November 17, 2020 08:18
Base automatically changed from video/annex-a to 20.12 December 10, 2020 05:51
@HansBusch HansBusch changed the base branch from 20.12 to 21.06 December 21, 2020 14:42
@HansBusch HansBusch removed the hacktoberfest-accepted Hacktoberfest contribution label Dec 30, 2020
# Resolved Conflicts in doc/Analytics.xml
…ename Direction to Mode and move up. Replace ResetTimerInterval by array ResetTime.
<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>
Copy link
Contributor Author

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

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

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?

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

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

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

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.

Copy link
Contributor

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.

@HansBusch
Copy link
Member

Thanks for spotting the inconsistency. Removed direction from occupancy counter example.

@HansBusch
Copy link
Member

Rename rule to "line crossing counting"

Copy link
Contributor
@kieran242 kieran242 left a 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

@HansBusch HansBusch merged commit 321eba5 into 21.06 Mar 11, 2021
@HansBusch HansBusch deleted the video/annex-a-counting-rule branch March 11, 2021 08:49
HansBusch added a commit that referenced this pull request Jun 23, 2021
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0