search

clamsproject / app-east-textdetection

Apache License 2.0
1 stars 0 forks source link

Change how time points are associated with bounding boxes #1

Closed kelleyl closed 1 year ago

kelleyl commented 3 years ago

Instead of generating an alignment annotation, add the id to bounding box annotation properties.

keighrim commented 3 years ago

I wonder if this is actually what we want. We might need to think an talk through how we want to handle alignment between image regions and time spans / points. Bringing @marcverhagen in to the dicsussion.

marcverhagen commented 3 years ago

With alignment I am constantly hopping from one approach to another: (1) an alignment as a full-blown relations between annotations, with all the bells and whistles that we may need, and (2) alignment as a property on one annotation that points to another. The second is simpler, but may be too simple.

Related to that we have had some discussions on what anchors do and I think the two intersect.

keighrim commented 1 year ago

While working on #4 (and #5), I found the proposal here was already the case (not generating TimePoint annotations) except the timePoints property were not actually used, instead frame property was used to record the frame number. (see https://github.com/clamsproject/app-east-textdetection/commit/5321c75#diff-7a283e35179cfc16f178c2af0777e7e39ba628bbc49d30da0f814b627869c290R132-L140 )

So when #5 is merged, we can consider this issue is resolved. However, I wonder this "bugfix" of mine will effect mmif-viz and wanted to bring @haydenmccormick into the conversation.

Also for the full context, the code linked above was updated with full support for other timeunits in the following commits in #5.

https://github.com/clamsproject/app-east-textdetection/blob/e7f2549fc875fd5c1762a3046de68ea67a82140d/app.py#L71-L81

keighrim commented 1 year ago

The 'frames' property has also been used in downstream apps, for example tesseract-wrapper.

linking these PR as reminders to fix.

keighrim commented 1 year ago

Given our recent discussion about setting a standard time unit (https://github.com/clamsproject/mmif/issues/192), I think we need to re-consider this stop-gap implementation (having timePoint property in BoundingBox object), and instead move to using more standard way of (TimePoint, BoundingBox, Alignment) triple to constrain all time-related anchor objects to TimePoint and TimeFrame instances only.

keighrim commented 1 year ago

fixed by #8.