Multicam Track Store Support

[BryonLewis]

Second PR into the multicam-support which will contain trackstore modifications to support multicam.

Currently the track store has been modified so that there is a base/default camera which is populated with the internvalTree for each item as well as a separate trackMap for for each camera contained with the camTrackMap. When a track is requested the camera name must be given for it to be transferred into the camTrack Map instead.

TODOs:

• Saving/Modification of specific cameras (this might be able to be done by automatically change the Camera dropdown based on the selected camera). Or saving needs to save the data across all of the tracks.
• Related to above is the creation of new tracks and having them associated with the selected camera
• Editing a selected track on a specific camera should allow drawing another annotation which gets added to the camTrackMap for that camera.
• Deletion of a annotation of a specific camera vs deleting the entire track?
• Import tool needs to be associated with selected camera
• Export tool also associated with selected camera

Based on the specific needs of multicam I'm wondering if there should be a another panel with some of the options that are listed above?

For pipelines we need to do a postprocess step which takes the highest existing trackId and starts adding values to it because if not it will overwrite the current values.

[BryonLewis]

Started working on this and decided to remove camTrackMap and have a single trackMap: Map<string, Map<TrackId, Track>> instead of a separate map for multi-camera. This means that a single camera has a 'default' camera in the trackMap. There are two types of accessing actions for trackMap those that require the drawing data like layers and those that need information at a higher level like begin/end times or confidence values. Typically these higher level data calls are done using getTrack(trackMap, trackId). I modified that function so it will return the first track it finds seeing as the confidence/begin/end times for all camera Tracks with the same TrackId should be the same.

[BryonLewis]

List of things which need to be dealt with/fixed.

• - New track creation followed by Escape (doesn't remove the existing track)
• - Editing mode on a camera which doesn't have a track but a track is selected
• - Keyframe toggle must work in conjunction with the current camera, removing a keyframe on one camera should remove the annotation. Adding a keyframe should allow drawing of the new item as well

[BryonLewis]

• - Need to review getTrack access across the entire system and ensure it is properly getting the right Track data.
  • Done, but still have this call to getTrack(trackMap, TrackId, 'any') for some items. Not liking it and may change in future.

[BryonLewis]

Pushed an update to clean up some of the getTrack requests.

Still needed are that sortedTracks -> enabledTracks -> filteredTracks -> eventData right now grabs only the first available track. This data path needs to know about the full track length and size so it accurately displays it in the event Viewer.

Editing tracks seems to work with being able to select a track and right click on a new camera to go right into editing/adding a track with the same ID. There may be some adjustments that are needed for that though.

Keyframe toggling works for the different cameras to remove the annotation. I need to make it so any track that has multiple cameras has the keyframe option. It doesn't work on a single frame with two tracks on each camera that are only on that frame. I.E. - detections that are on two cameras.

So finally:

• Cleaning up Event Viewer display for multi-camera tracks
• Cleaning up keyframe toggle for single frame detections where there are multiple tracks (I use this is my ability to delete a single track on a specific camera)
• Get testing to work properly and make sure this didn't break anything for single cameras.

[BryonLewis]

@subdavis - Just have this for review/comment before you start taking a look at the actual code.

Core Idea
With multi-cam data TrackIds are a unifier between the multiple cameras. A TrackId is used to index tracks among multiple cameras. I.E TrackId: 1 can have tracks in multiple cameras at different frame times. We need to be able to visualize, edit, create and modify these tracks and relationships between them using DIVE.

CamMap - new trackMap for each Camera. By default there is a ‘singleCam’ camera name if there are no multi-cams. If there are multi-cams the ‘singleCam’ will be removed after loading if it doesn’t exist as a name for the multi-cams.

Track Access - Getting and using Track data has changed among multiple cameras because of different request access. Sometimes you need a track to edit it specifically and want the system to error if it isn’t found. Sometimes you just want to see if Any track exists for a camera trackId. Sometimes you want all of the tracks for all cameras across all Ids.

• getTrack - Will retrieve a track for a camera name if specified. Will error if a track for the specific camera doesn’t exist.
• getAnyTrack - will get the first track for a specific trackId found across all cameras. Usually this is used to just to confirm the existence of a trackId in the trackstore, or it may be used for simple info like the confidence pairs (which should be the same across all tracks)
• getPossibleTrack - will get the track for a cameraId or will return undefined if a track doesn’t exist. This is useful for automatic creation of track in other cameras while in edit mode.
• getTracksAll - will return an array of all of the tracks across all cameras. Used to set values on all cameras like confidencePairs or track Attributes
• getTracksMerged - will clone the tracks into a merged track. This is used for the sortedTracks so that we have an accurate keyframe display of all tracks that can be used in the event viewer. Also enable the track seeking as well.

SortedTracks

Required a modification to sortedTracks so that the event viewer had the proper display of track frames when selecting a track. The track in the event viewer and for sortedTracks needs to be a culmination of all camera tracks for a specific trackId. I accomplished this by turning off notifications cloning and merging the tracks into a single Track and using that for sorted Tracks. It will keep its necessary data for frames, confidence pairs. This just required some updates to make sure that nothing is being directly operated on the sortedTracks data and it uses the camMap plus one of the getTrack functions to do any modifications. I am open to more explicitly doing this instead of relying on hacking the merge function together to get this information. Perhaps sortedTracks is not even an array of Track[] but of some more limited data-type only providing the information needed to consumers.

Loading/Saving Track Data:

• Loading - I’ve already stated that by default there is now a camMap which holds each camera and a default camera called ‘singleCam’ which is used if there isn’t multiCam data.
  During loading it will just load each camera in the multicam list. After it is complete it will remove the default camera if there is no camera named ‘singleCam’ in the list. For creating the merged tracks for sortedTracks I needed to disable the track.notifier from being triggered so there is an additional data item added to track to prevent triggering this.

• Saving - pendingSaveMap has now been abstracted to another layer out like the camMap where there is a pendingSaveMap for each camera instead of a global one. While there is still a single count which is used for display of number of changes, clicking on save will save each {datasetId}/{cameraName} with its proper upsert/delete values. This should revert to the default behavior for a single camera.

Camera Tracks and Editing

Camera Selection

There is the idea of selecting a camera as well as an individual idea of selecting a track. For cameras a camera must be selected at all times. There is no time in which a camera isn’t selected except the initial loading time. Afterwards a camera is selected and actions in multicam mode will be applied to the selected Camera. This information is placed in the ‘provides’ section because it is used globally for track access and modification. Clicking on a different camera or using the camera select dropdown will select different cameras. Right clicking on a camera while having a track already selected will place the track in the camera into editing mode or creation mode if one doesn’t exist.

Track Selection

Selecting a Track is specifically related to it’s trackId. So clicking on a track in a camera or selecting from the TrackList will select a trackId. When a trackId is selected all tracks in all cameras are highlighted as being selected. Again, Right Clicking on a new camera while a track is selected will either select the track in the camera for editing or generate a new track for that camera. Clicking another camera or hitting escape will back out of this.

MultiCam Tools

We need some tools to do specific interactions with a multicam track. These tools only work once a track is selected and they provide the following information tools.

• Existence of Detection for the current frame across all cameras
• Existence of a Track for each camera with the selected trackId.
• The ability to remove the current detection or the current track from any of the cameras.
• Ability to split off a camera Track from the main trackId into a new TrackId
• The ability to add/edit a detection for the current frame for any camera
• The ability to link a track from a camera into the currently selected TrackId
• This can only be done if there is no existing track for that camera
  • This has caveats where the track being linked needs to only exist in that camera. This prevents conflicts of trying to link tracks which both have data on multiple cameras.

The reactivity of this tool/panel needs to be updated. I don't have that fully configured yet.

Camera Synchronization

This was a feature asked for to be able to synchronize camera movements between mutliple cameras. This is specifically for the case of IR/RGB imagery and being able to zoom in and look at the same spot. I just added a way to synchronize the cameras a basic toggle. Open to the presentation of the icon, maybe a camera-lock button instead of the bade I went with. The camera sync should align mutliple cameras. It is a bit beta in the fact that zooming out all the way may have a different effect but it can be fixed with a camera reset.

Single Camera Pipeline Runs
One of the features was the ability to run single Camera pipelines on a camera in a multicam system. There are a bunch of issues with trying to integrate a single pipeline into multi-cam data like trackId conflicts would happen and type conflicts. The suggestion was to indicate that there would be problems and tell/force the user to delete all their data. So I modified the runPipeline when in multicam mode to indicate to the user they should delete their tracks before running a single camera pipeline. There were also some minor modifications to ViewerLoader to make sure that there are proper notifications of single pipelines completing.

[subdavis]

Thank you for writing out this summary, it will be a great guide to understanding the review. I'll probably see if we can convert some of it into inline comments where appropriate.

Saving multi-cam tracks

This might be unnecessary at this point in time, but I always get a little uncomfortable with client-side update orchestration. I think annotation saves should be atomic operations (at least as far as the client is concerned) where the interface maybe now includes an upsert/delete list for each camera and the backend figures out how to deal with it. No action necessary, just wanted to mention it.

I read and mostly understand the rest of the comments. When you want me to review, I'm going to be on the lookout for performance issues or added runtime complexity in some of the new data structures.

[BryonLewis]

I read and mostly understand the rest of the comments. When you want me to review, I'm going to be on the lookout for performance issues or added runtime complexity in some of the new data structures.

There are some things that may have me a bit worried about runtime performance at least while in multicam mode.. Mostly the deepclone of merged tracks in the case of multiCams for the sortedTrack list to provide a reactive tool for things like attributes/type lists and track Items.

[subdavis]

I'm pleasantly surprised by how straightforward this all is. I expect this will be a fairly quick review, so awesome PR!

Before I get too deep on the code review, I want to discuss one high-level design idea. Now that I see the implementation, I think getTracksMerged may not be necessary.

As I understand it, you need this for 2 things:

1. To order the track list by start time. Merging is the right way to do that, IMO, so the behavior is fine.
2. To generate the event timeline stuff. I'm going to argue here that the new merge behavior is wrong, and that the event area should show the specific start/end and keyframes for the selected camera and re-draw when selected camera changes. For interpolation, seeing keyframes from other cameras is definitely misleading. And the interpolated ranges will almost definitely be wrong. I think the start/end bounds aren't as much of an issue, but I still think showing per-camera timelines is the right way to go. If we need to do some optimization here such as keeping all 3 timelines rendered all the time (like keep-alive) we can do it later hopefully?

What do you think about that?

Out of scope

But I still want to mention it:

• I still think some things from the timeline need to be duplicated for each camera, such as the image file name. This might mean another thin toolbar that appears below the camera area in multicam configurations.
• For video, in the future, I think we need to support multicam with different framerates on different cameras, and probably some kind of synchronization measure to say that "Frame 0 in cam 1 is frame 10 in cam 2". I think you'd be hard-pressed to get actual alignment in real-world conditions without something like this, but I could be wrong

[subdavis]

Seems like getTrackAll should throw an error if it can't find any tracks with that ID.

[subdavis]

This is a class with reactive properties and notification functions and stuff. This line gives me a lot of anxiety.

[subdavis]

You haven't used the optional args in this file, you can remove them from the interface.

[BryonLewis]

The image filename is displayed for the selected camera. If you change your selected camera it will show the image filename for that camera. I don't know where we could fit all N images file names for each frame.

The merging of tracks provides the stuff you mentioned above. It also allows frame seeking within a track to any frame which has a detection on it. Specifically those frame seeking buttons on the trackItem in the track list.

I was basically avoiding creating a new data format which can be used in replacement of the sortedTracks. I think we may need to design something that can be used for all the needs mentioned. Interpolation UI is disabled in multi-cam mode right now because of that complexity. I could see making a breakdown of each camera's tracks in timeline, I need to consider usage for having separate timeline keyframe views for each camera vs having a single timeline. The idea of these tracks are that they are representing one object/creature across multiple cameras and that it can be contained on two overlapping cameras or it can drift into one camera at a time. Having separate annotation data (meaning interpolation and being able to view the keyframes for each camera) might be harder to get a sense as to what the track is doing. Maybe there is a middle ground where the timeline selected view indicates if a keyframe is present on multiple cameras vs a single camera?

[subdavis]

Got it, thanks. I agree that a new data structure may be necessary, but since you've disabled interpolation UI in multicam, let's punt on it for now. Using the merge to get what you need is the obvious easiest way. We can revisit performance later. We can also think about visualization later.

No change necessary, I'll continue to review.

[subdavis]

UI Usability

This is a round of feedback not related to code.

Feedback

• Synchronize controls only works on the selected camera. Scrolling on off-cameras or dragging on off-cameras doesn't work. I think it should.
• I don't personally like the sync camera icon. I think you should just use the Link/unlink toggle instead of the camera with a link badge. The badge looks kinda weird.
• It's not possible to know which camera is which, making the multicam tools quite hard to use.
• Delete confirmation needs a cancel option. Pressing escape should not disrupt the selected track if I click delete then press escape to cancel. It's rather disorienting to have the whole UI reset when I press escape to dismiss the delete dialog.
• Multicam tools could probably show a more helpful message when nothing is selected, similar to the "This panel is used for" message in track editor, with instructions to select a track to activate the panel.

Bugs

• Multicam Tools:
  • Add detection to another frame doesn't seem to work at all if you're in edit mode. I end up in "Create rectangle" mode, but with no crosshair to draw.
  • Add detection to another frame actually deletes the detection in the original frame if you press escape to get out of track creation mode.
    • Select track. Add detection for another camera. Press escape. See track get deleted.
  • You have to click edit twice when you want to edit a track on an unselected camera.
• I've corrupted a dataset somehow by deleting all its annotations and then creating a new one. Now I get vue.runtime.esm.js?2b0e:1897 TypeError: Cannot read property 'annotationExists' of undefined. I can't reproduce it, but there's definitely a bug somewhere. Reloading the dateset didn't help. I'll keep trying.
• Switching cameras in creation mode causes annotation without geometry to be created.
  • Select cam 1. press n. Click into camera 2. See no crosshair. See crosshair still in unselected camera 1. If you draw, nothing happens.
• r to reset windows is broken.

[subdavis]

Round 2

I'm submitting another round of bugs while I continue to look at code.

Bugs

• Unlink appears broken. Create 2 or more linked tracks and attempt to unlink:
  • Error: No camera Map with the camera name: 2 in console.
• Something is weird about right-click across cameras.
  • Create a track in a camera. From edit mode, right-click into a non-selected camera. You are now in edit mode in a different camera. This is cool, can't tell if intentional. Draw a track, and do it again in a third camera. You end up with 2 tracks linked and 1 independent. Not sure what's up there. [1]
• After deleting a track in an alternate camera, the delete button is still here. If you press it again, you get errors.
  • Error: TrackId 749 not found in trackMap with cameraName LEFT
• There's an inconsistent behavior where dragging in an unselected camera sometimes selects it and sometimes doesn't depending on where the mouse release happens. If you drag an unselected and the mouseup happens over the unselected, it becomes selected. If you mouseup elsewhere, it doesn't become selected.
• Lock camera to selected track broken on stereo

[1] https://user-images.githubusercontent.com/4214172/160863095-3e7fa8ce-9389-463b-be5c-933fae626457.mp4

Other feedback for later

No action, just want to record this as I discover stuff.

• I would like to move some of the new functions out of Viewer, like linkCameraTrack and unlinkCameraTrack. That's too much random handler logic in Viewer.vue IMO.
• If you have the multicam tools open, then exit out and open a single-cam dataset, the multicam tools are still open.
• (Unrelated) in event list, track events that need to be scrolled into view need an offset of their height applied. Tracks where the top of the track bar aligns with the bottom of the viewport will not get scrolled up. There should be a difference in behavior when scrolling up into view and down into view.

[subdavis]

Could you elaborate here? What events does this dependency capture?

[BryonLewis]

I'll update the comment. Using the other values aren't enough because creating/removing a track from a camera won't update SelectedCamera or SelectedtrackId if there are remaining items. EnabledTracksRef is based on the merge across tracks so the actions in the multiCam will be updated whenever there is a change.

[BryonLewis]

Going to correct myself, I think this only works in the case of linking/unlinking tracks. I need some other indicator for deletion of a detection/track from a camera.

[subdavis]

What is this lastIndexOf thing doing? Should we have a utility function?

[BryonLewis]

The datasetId may be a multicam Id: datasetId/cameraId. This allows use to run the single camera pipelines on the selected camera. When running a multiCamPipeline we want to run it on the root so it knows to grab all cameras. So it's chopping off the cameraId datasetId/cameraId to datasetId.

[BryonLewis]

Commented on in the order they appear.

Bugs:

• First bug was a mis-match with vue template function arguments, I've updated it so unlinking should work
• Second bug fixed: again it was an overrideTrackId of 0 and not using || instead of the ?? operator. This behavior is intentional to make it easier to create tracks across multiple cameras. A right click on a different camera when a track i selected will kick you straight into creation mode for that camera and trackId.
• I hate it but the quickest short term I could do is canary the deletion of the tracks/detections to cause it to update the information. I believe that enabledTracks still needs to be in there for links/unlinks.
• I updated camera selection so that it should occur on mouse down instead of mouse up. Hopefully this doesn't interfere with other actions too much.
• • Lock camera was an issue in the mediaController which was mapping the toggleCameraLock function and calling (N X number of cameras) so on even camera numbers it would toggle on then immediately off.

Feedback:

• I did consider this at one point and it's something that can be done in the future. I believe I was modeling it after the trackSplit and it requiring some insertTrack and other non-handler abilities. I think they can probably all be moved into a utilities or trackManipulation file.
• This was something I noted in the Revision History PR. I didn't add an issue for it but I'm guessing I should now. Revision History UI #1188 (review)
• I'd add a global issue to fix this.

[subdavis]

Feedback

Huge improvement over the last revision. You fixed a lot of behavior issues, particularly right-clicking between cameras to select and keep edit mode on.

Code is looking great, I think this is really close to being ready to merge.

Issues

• I'm still hitting intermittent console errors, usually "Track # cannot be found in trackMap", but when I try to retrace my steps to reproduce them, I can't.
• +Types and editing type styling don't trigger a markChangesPending so they aren't able to be saved on stereo/multicam. Still works on single cam.
• Not quite a bug, but switching camera with left-click while in creation mode leaves you with an orphaned detection (no geometry) in the track list.
• I've got a family of bugs related to a create-then-link workflow. I tried to reproduce 3 times and ended up with 3 different bugs. [1] [2] [3]

[1] https://user-images.githubusercontent.com/4214172/161131219-3de8c53d-562f-4c91-b130-957714add87c.mp4

[2] https://user-images.githubusercontent.com/4214172/161131221-29c3e1ea-d149-4af6-afa1-4e30d316cb74.mp4

[3] https://user-images.githubusercontent.com/4214172/161131223-53cd07a5-85be-4541-9c6a-1f9576d3cc09.mp4

Other conccerns

Big picture, I'm a little concerned that the annotator is now so fragile that this will be the last significant feature we are able to add before a complete redesign is necessary. Once we feel satisfied that the annotator works, each future change risks significant regression bugs.

Things weren't in great shape before multicam, and I'm worried that this is pretty much it in terms of what the current architecture will allow.

[subdavis]

Why doesn't this work? If I have tracks linked on B and C, and I try to link either of those with A, they should become all one thing?

I understand if it was just too much of a hassle to implement, but it seems like this should work.

[BryonLewis]

Track 1: has tracks on [A,B].
Track 2: has tracks on [A,C].

You want to link Track2-C to Track1. I want the user to manually have to go in and split off Track2-C before they can link it. If not what do I do with Track2-A, do I delete it or automatically split it off. I wanted to make sure that if you are joining a link it needs to be a clean Track only on that camera.

[BryonLewis]

I should probably make it clearer by saying they need to split off the track before linking.

[subdavis]

I'm still kinda stressed about this. If every camera doesn't save correctly, your dataset is pretty much hosed. You could handle this on the backend and handle the case where something goes horribly wrong by dumping the failed update to file and rolling back the change.

Not something to handle now, just one more thing to worry about.

[BryonLewis]

Are you worried about overblasting the endpoint and having an issue with that? Or just that the dataset depends on N-camera datasets which increases the risk of failure?

[subdavis]

Should the default selected cam be singleCam?

I'm getting a little confused by all the defaults.

[BryonLewis]

ViewerLoader is outside of Viewer. Cameras only get selected and changed if there are multiple cameras. This means the datasetId remains standard for the case in which the camera isn't changed. The modifiedId which is used for running pipelines is dependent on the selectedCamera truthiness. If it remains '' then it uses the base Id. This allows standard cameras to operate normally for pipelines and then when you have multi-camera datasets it will use the selectec camera's Id: {datasetId}/{cameraName}. There is probably a better way to convey this in the code than how I did it.

[subdavis]

We now have an id, a modified ID, and a compound ID, and I don't know what any of them are. The variable names are kinda useless. Could each of them get a comment explaining what it is?

[BryonLewis]

I'm not entirely sure where compoundId came from, I'll clean this up to hopefully make it clearer.

[subdavis]

Should one of the trackStore helper functions (getTrack maybe) be used here?

[BryonLewis]

I don't think so, because the helper functions act on a camMap not a trackMap. Each LayerManager splits of it's own trackMap for that camera so it only knows about it's own tracks for drawing. It's done at the top of the LayerManager using the props.camera

[subdavis]

I'm not sure I understand this comment. I see that a GeoJS redraw is needed when camera change because you might want to turn the editing layer on or off, but this doesn't seem like what the comment is talking about.

[subdavis]

Round 4

This resolves all the bugs from the previous round.

I hadn't tested pipeline stuff up until now. Few issues there, though I expect they're mostly straightforward.

Bugs

• On a track that exists in only one camera, place into edit mode. Right click a different camera, you're now in drawing mode. Press any unlink button (which should not be active). See a new track without geometry created [1]
• I have a multi-cam dataset with 2 cameras, but the 3-cams pipeline appears as an option. I suspect if I had 4, the same would be true. The UI allows me to run a 3-cam no matter how many cameras are in my multi-cam data
• Running a measurement pipeline on a stereo dataset shows the warning as if it were single-camera. If I click run-anyway, I get Error: Error invoking remote method 'run-pipeline': Error: Attempting to run a multicam pipeline on non multicam data.
• Linking doesn't work when bounding boxes are turned off. [2]

[1] https://user-images.githubusercontent.com/4214172/161299128-4e736b83-bffd-42f2-8c7e-b2469ebc0409.mp4

[2] https://user-images.githubusercontent.com/4214172/161298844-cec890a6-756e-48e5-ab0e-b40235001b96.mp4