Initial Stereo/MultiCam Support Commit by BryonLewis · Pull Request #803 · Kitware/dive

BryonLewis · 2021-06-10T16:27:31Z

I broke up the massive PR into smaller ones.

This begins with allowing upload of stereo data and running of the latest stereo pipeline.
It treats each folder as it's own dataset and modifies some of the subType and metadata to make it easier to filter.
As of now the only view is the default camera view chosen at the beginning and it behaves like the main multicamera view so you can only the measurement pipeline on it
If you run the main stereo pipeline you will get results. If you check project folder for each camera you should see JSON files for each camera's results.
I updated the processing of annotations after a pipeline to include the suggested attributes so they will display properly in the UI. This is how you get measurements back.

This is laying the foundation for being able to display the multicamera view as well as import annotation files through the UI.

After this I think I would be overhauling the import dialog so allow for individual annotation files per camera and I think I may want to have a discussion about import Dialogs and what we want to use.

f4str · 2021-06-11T20:22:23Z

I found a minor UI bug where the buttons are different sizes

Looks like if the width is between ~1000px and ~1200px this happen.

I might be nitpicking here, but it didn't seem very intuitive when I got to this popup box for importing multicam

It took a second to realize you need to submit a name then you get the choose file prompt. It might just be me though.

Other than that, everything else ran just fine on my end.

BryonLewis · 2021-06-11T20:39:37Z

It took a second to realize you need to submit a name then you get the choose file prompt. It might just be me though.

You're right, combination of (in my opinion) the bad text input default style of vuetify elements and me becoming complacent after testing it a bunch.

BryonLewis · 2021-06-15T13:08:54Z

@f4str - thoughts?
Gave the button a description and used the outlined version of the folder/name fields to make it more clear that it is a text input area.

f4str · 2021-06-15T13:24:39Z

I think that looks a lot better. It's a lot more intuitive now that you need to set a camera name first before uploading any images/videos.

f4str

Looks good to me.

aashish24 · 2021-06-22T15:27:20Z

expected date for merge: June 29th as per our discussions

subdavis

I have just a little more to look at (multiCamUtils.ts) but I keep getting distracted and I want to throw this over the fence.

subdavis · 2021-06-18T13:18:27Z

client/dive-common/apispec.ts

+    imageData: FrameImage[];
+    videoUrl: string;
+  }>;
+  display: string;


Comment what this is. Also note if it will be removed once there's true multi-cam support?

subdavis · 2021-06-18T13:20:11Z

client/dive-common/apispec.ts

+  display: string;
+}
+
+interface CustomMediaImportPayload {


Document what this is. Also, I tend to think of "Payload" as an import. Could we rename this to "MediaImportResponse" or something? I'm not sure "Custom" adds any meaning.

subdavis · 2021-06-23T20:18:52Z

client/dive-common/apispec.ts


 type DatasetType = 'image-sequence' | 'video';
 type MultiTrackRecord = Record<string, TrackData>;
+type SubType = 'stereo' | 'multicam' | null;


"DatasetType" and "SubType" seem backwards as names?

A dataset could be stereo, multicam, or normal. Each camera can have a subtype image-sequence or video?

Perhaps subType should be DatasetType and DatasetType should be MediaType?

Dataset is image-sequence | video | multi - I didn't add the multi to the common/apispec because it isn't supported on the Web side yet. I could add it but it would require some updating to other files outside of the desktop scope to support it. Maybe I should place a comment here?

So I could see having DatasetType renamed to MediaType where it has image-sequence | video | multi(desktop only) I think I would want to rename it on the Server backend in python as well at the same time (a lot more files touched).

SubType may more accurately be renamed as multiCamType? There are differences in pipelines than can only be run based on the subType. Stereo only pipelines vs Multicam only pipelines. This allows me to disable/enable pipelines without having to reach far down into the meta and create a computed property (especially in the future folder view on web). I could move this into camera object although we may want to use this same field for for future identification of specialized media outside of MultiCam subtypes. Like IR images may have special pipelines on them and can be designated using this field.

The secondary use of it is inside of the JSONMeta for the displaying of the icon indicating that the dataset is stereo (binocular icon) or multicam (burst image icon).

So I can move it into the camera field and remove the icon display capability in recents because I wouldn't know the type until I load the fully metadata Or I could rename it to something like multiCamSubType or multiCamType.

subdavis · 2021-06-23T20:19:17Z

client/dive-common/components/RunPipelineMenu.vue

      type: Object,
      default: () => ({}),
    },
+    subTypeList: {


mediaTypeList?

subdavis · 2021-06-23T20:23:37Z

client/platform/desktop/backend/native/common.ts

  // const allowedPatterns = /^detector_.+|^tracker_.+|^generate_.+|^utility_|^measurement\.gmm/;
  // Disable measurement for now.
-  const allowedPatterns = /^detector_.+|^tracker_.+|^generate_.+|^utility_/;
+  const allowedPatterns = /^detector_.+|^tracker_.+|^generate_.+|^utility_|^measurement_gmm_.+/;


Remove the comments above?

subdavis · 2021-06-23T20:57:27Z

client/platform/desktop/frontend/components/Recent.vue

-      if (!recent.multiCam) {
-        if (recent.type === 'video') {
-          return 'mdi-file-video';
+      if (recent.type === 'multi') {


Should you be checking subType here?

right below that I check the subType if it is of type multi to either display the binoculars or the burst photos.

subdavis · 2021-06-23T21:00:18Z

client/platform/desktop/constants.ts

+    transcodedImageFiles?: string[];
    transcodedVideoFile?: string;


nit: in DatasetMeta, the transcoded media keys are not nullable, they're empty string and empty array if unused. For consistency (and my poor brain) it would be nice if these matched.

subdavis · 2021-06-25T14:13:26Z

client/platform/desktop/backend/native/viame.ts

+      command.push(`-s ${arg}="${file}"`);
    });
+    multiOutFiles = {};
+    Object.entries(outFiles).forEach(([key, val]) => {


key val => cameraName, fileName

subdavis · 2021-06-25T14:18:09Z

client/platform/desktop/backend/native/multiCamUtils.ts

  let destLoc = '';
  if (jsonMeta.multiCam) {
+    let breakFlag = false;
    Object.entries(jsonMeta.multiCam.cameras).forEach(([key, val]) => {


key, val => cameraName, camera ?

subdavis · 2021-06-25T14:19:44Z

client/platform/desktop/backend/native/multiCamUtils.ts

+    let breakFlag = false;
    Object.entries(jsonMeta.multiCam.cameras).forEach(([key, val]) => {


Don't use forEach proto method in loops you need to break from, prefer for...of and break keyword

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of

This can be done but ESLint gets upset and requires // eslint-disable-next-line no-restricted-syntax mostly because it doesn't want you to use a for of like that. I think the for...of is cleaner than the break flag I was using which was causing additional iterations.

I also usually forget that for...of and for...in require RegeneratorRuntime, which I avoid like the plague.

You might just need a regular for loop here?

subdavis

Semi-populated form issue

When you start to work on a multi-cam import, and you create some named cameras, then you switch to keyword filter, you end up in a weird state where both exist. Same happens for stereo and multi.

Stereo requires calibration file (no change requested)

I'm not totally sure if it should. I understand that the stereo pipeline requires it, but maybe that should be an error thrown when the pipeline runs? Will anyone want to use stereo without the calibration file?
I think this is fine to leave alone as well, just wanted to leave a breadcrumb about this

BryonLewis added 2 commits June 10, 2021 12:21

initial stereo commit

c35acd4

using the actual marker

6d5365d

BryonLewis requested review from f4str and subdavis June 11, 2021 16:49

updating some styling

fe738bf

f4str previously approved these changes Jun 15, 2021

View reviewed changes

Merge branch 'main' into desktop/stereo-init

56e2c2c

BryonLewis dismissed f4str’s stale review via 56e2c2c June 22, 2021 12:53

subdavis suggested changes Jun 25, 2021

View reviewed changes

BryonLewis added 3 commits June 25, 2021 14:17

Merge branch 'main' into desktop/stereo-init

c9668fe

addressing comments

2687f0e

fixing mistakes

030dcff

subdavis self-requested a review June 30, 2021 01:12

subdavis suggested changes Jun 30, 2021

View reviewed changes

adding in clearing for swapping import types

bdd7b2b

subdavis self-requested a review July 1, 2021 00:51

subdavis approved these changes Jul 1, 2021

View reviewed changes

BryonLewis merged commit bae90aa into main Jul 1, 2021

subdavis deleted the desktop/stereo-init branch July 1, 2021 12:18

subdavis mentioned this pull request Jul 20, 2021

[FEATURE] Ability to select camera in multi-camera dataset. #851

Closed

subdavis mentioned this pull request Dec 9, 2021

Example without fetching metadata #1088

Merged

		transcodedImageFiles?: string[];
		transcodedVideoFile?: string;

		let breakFlag = false;
		Object.entries(jsonMeta.multiCam.cameras).forEach(([key, val]) => {

Conversation

BryonLewis commented Jun 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

f4str commented Jun 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BryonLewis commented Jun 11, 2021

Uh oh!

BryonLewis commented Jun 15, 2021

Uh oh!

f4str commented Jun 15, 2021

Uh oh!

f4str left a comment

Choose a reason for hiding this comment

Uh oh!

aashish24 commented Jun 22, 2021

Uh oh!

subdavis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

subdavis left a comment

Choose a reason for hiding this comment

Semi-populated form issue

Stereo requires calibration file (no change requested)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BryonLewis commented Jun 10, 2021 •

edited

Loading

f4str commented Jun 11, 2021 •

edited

Loading