Skip to content

Electron#397

Merged
subdavis merged 4 commits intomasterfrom
electron
Oct 30, 2020
Merged

Electron#397
subdavis merged 4 commits intomasterfrom
electron

Conversation

@subdavis
Copy link
Copy Markdown
Contributor

Electron desktop client

Screenshot from 2020-10-15 17-39-24

@subdavis subdavis mentioned this pull request Oct 21, 2020
@subdavis subdavis force-pushed the electron branch 3 times, most recently from 06ff6b1 to 75a6635 Compare October 27, 2020 20:27
@subdavis subdavis marked this pull request as ready for review October 27, 2020 20:34
@subdavis
Copy link
Copy Markdown
Contributor Author

subdavis commented Oct 27, 2020

@BryonLewis @AlmightyYakob I'd like to go ahead with merging this. I don't want to diverge from master too far.

Right now it can just open media. No annotations, no save. A lot is missing. I think this is a good incremental addition.

Thoughts?

@BryonLewis
Copy link
Copy Markdown
Collaborator

Thoughts?

I'll take a closer look at it tomorrow. I think that having this will help immensely in considering the desktop version and how any future changes could impact it. Part of the idea of having the styles/attributes in a json file located within the dataset is to make it a bit easier for the desktop version as well as exporting from web and importing into Viame Heavy (or the opposite). Less reliance on girder as a service and possibly more reliance on it as a sort of filesystem. I haven't thought through all of it yet, but this will give me some stuff to consider.

Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this wasn't intended to be tested on Windows but I did anyways. It builds an executable and runs but it seems as though there may be some issues with the folder dialog (Not having a filter so I couldn't see individual image/video files, may be the way the 'directory only' setting behaves) and serving of the images (Looks like it gets the number of frames for the images correct but nothing is displayed). I can take a further in-depth look at it in the future as it could be local windows firewall settings.

Couple of minor things that may change in the future anyways:
Structural:
I don't think we should stick to solely folders for opening datasets. I think we can support opening directories as well as opening individual files. I understand that the Web version is using 'folders' for it's dataset but it's technically a girder folder. For importing data we use individual files which allows you to select a subset of the image frames. Or a single video file in a folder which contains multiple videos. (That's how I noticed, I keep my videos together and went to select one but it wouldn't work).

Very Minor:
This is something I only noticed now because of the length of some of my paths. It should be changed in the Web version as well. The "Launch Annotator" should be a column in a row instead of appended onto the end of the filename. Long file paths (desktop) and folder names (web) cause a wrapping which places the "Launch Annotator" in different places and just doesn't look right.
Very very minor: That default app Icon in ubuntu is nasty.

I have other issues that are related to support with the web version.

Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of the other stuff works, but I think this issue is a significant one that needs to be edited/fixed so that the web version works.

@subdavis
Copy link
Copy Markdown
Contributor Author

I think we can support opening directories as well as opening individual files

This will complicate things. Suppose you have 100 images in a folder. You open 40 of them as one project. You open a different 40 as another. What do you call these projects? How do you indicate which is which? Where do you keep track of these meta-project files, because they can't go in the image directory now. What about annotation files -- you can't know which annotations.csv goes with which subset.

I don't want to support opening individual files unless the customer specifically asks for it. It's not that these problems are unsolveable, they just require extra code and extra testing, and we already have a lot going on.

The desktop app works exactly the same as the web version. You can't put multiple videos in a folder and then open them individually. A folder is a dataset.

With the web version these aren't available on the meta object

Yeah I'll have to fix that

Windows issues

I think we should move the path to a query param though: localhost:2345/api/images?path= seems more appropriate. I'll try this soon.

@BryonLewis
Copy link
Copy Markdown
Collaborator

I think we can support opening directories as well as opening individual files

This will complicate things. Suppose you have 100 images in a folder. You open 40 of them as one project. You open a different 40 as another. What do you call these projects? How do you indicate which is which? Where do you keep track of these meta-project files, because they can't go in the image directory now. What about annotation files -- you can't know which annotations.csv goes with which subset.

I don't want to support opening individual files unless the customer specifically asks for it. It's not that these problems are unsolveable, they just require extra code and extra testing, and we already have a lot going on.

The desktop app works exactly the same as the web version. You can't put multiple videos in a folder and then open them individually. A folder is a dataset.

This is my mistake, I forgot that the web version is 'copying' your local data to individual folders instead of just referencing it like the desktop version. What you're saying makes a ton sense keeping that in mind.

@subdavis
Copy link
Copy Markdown
Contributor Author

subdavis commented Oct 30, 2020

Addressed comments in d5ff033.

I squashed a bunch of commits as well.

  • Windows issues should hopefully be fixed now, but can't test.
  • Issues with loading datasets on web version are fixed.
  • Changed to /api/media?path=
  • Added a trivial vanilla router for node's builtin http server. I briefly considered adding an express.js dependency for robustness, but it seemed like way overkill.
  • Added mime type filtering, for security or whatever /shrug/

async function loadMetadata(datasetId: string): Promise<DatasetMeta> {
const ds: VIAMEDataset = await root.$store.dispatch('Dataset/load', datasetId);
return ds.meta;
const dataset: VIAMEDataset = await root.$store.dispatch('Dataset/load', datasetId);
Copy link
Copy Markdown
Contributor Author

@subdavis subdavis Oct 30, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type coercion is a lie. dispatch() returns an any because we aren't doing proper typescript vuex.

Not a big deal, but also why I don't like vuex. Maybe at some point Jake can show us the righteous path.

Copy link
Copy Markdown
Collaborator

@BryonLewis BryonLewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the previous issues are all fixed. I did a fully docker build and the new version seems to run and load properly with pipelines, saving and everything else.

Windows desktop now can load images/videos properly with either a built version or a yarn serve:electron - built version is a self extracting exe ~55MB in size.

I think at some point we may want to look into a setting that allows Windows users to see the file in a folder they are selecting. Right now it uses a standard file browser but you can only see folder names and if you open a folder you don't see any files inside of it. I imagine there is some workaround for this which I can look into in the future.

Side Note: thanks for the HTML5 Video link, didn't know the history behind ogg and web video.

@subdavis subdavis merged commit b268bd1 into master Oct 30, 2020
@subdavis subdavis deleted the electron branch October 30, 2020 12:58
BryonLewis added a commit that referenced this pull request Nov 16, 2020
Trying to get the styling and layout setup

Updating some styling

Updated styling a bit and added toggling

Started updating the editor and connecting it to the attributes endpoint

Updating styling and fixing various issues

mend

mend

Electron (#397)

* add electron

* Add missing api methods

* Fix build errors for electron, add to CI

* Address comments

Update deploy to only Thursday

Update base image for girder worker

Fixes FPS issue and image caching (#419)

* fixing fps and modifying cache

* mend

Update docker docs (#379)

Co-authored-by: Brandon Davis <git@subdavis.com>

Update blank.yml (#415)

Upgrade to node LTS

Co-authored-by: Jacob Nesbitt <jjnesbitt2@gmail.com>

Add checkbuild.sh (#425)

Add load and save for JSON annotations (#414)

* Add load and save for JSON annotations

* Relax json file name requirement

* SIP

* Implement settings, sanity checks, separate loading of datasets

* WIP

* remove settings page

Fix deploy.yml

Update day strings (#430)

Fix typo in library README

Allow setting of arbitrary attributes on video player (#435)

* Mute video prop

* Allow setting arbitrary attributes

Downgrade urllib3 (#439)

Fix inverse interpolation (#433)

Run pipelines enabled incorrectly (#441)

Minor fix to attributes

Minor ui fixes

Addressing changes

Enable pinch zoom on mobile (#440)

Co-authored-by: Brandon Davis <git@subdavis.com>

Add dummy generators (#438)

conversion to using api for attributes editing

converted attribute editor and added in additional editing

Updating UI interactions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants