Skip to content

Allow setting of arbitrary attributes on video player#435

Merged
subdavis merged 4 commits intomasterfrom
client/mute
Nov 11, 2020
Merged

Allow setting of arbitrary attributes on video player#435
subdavis merged 4 commits intomasterfrom
client/mute

Conversation

@subdavis
Copy link
Copy Markdown
Contributor

@subdavis subdavis commented Nov 11, 2020

Example

<video-annotator :video-player-attributes="{ muted: true, loop: true }" />

You can absolutely footgun with this. But I wanted to expose all possible configuration rather than having explicit props for everything.

I put it below src, preload to allow hacking on those, but above onloadMetadata because that would be unreasonable to try to hack.

@subdavis subdavis requested a review from BryonLewis November 11, 2020 14:30
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 understand the problem with shooting yourself in the foot with this.

Is this intended for vue-media-annotator instead of VIAME-Web in general. Is it solely to get around the fact that we don't have audio controls or are there other features you want added in (looping)?
I.E: Looping, if specified in shouldn't we make the left/right arrows do the same looping action then?

I don't mind this is as a quick fix to solve specific needs. I think if it is used too often we may want to think about specifically supporting audio controls as well as looping behavior (in terms of key controls)

@subdavis
Copy link
Copy Markdown
Contributor Author

This is a convenience for library consumers.

I don't mind this is as a quick fix to solve specific needs. I think if it is used too often we may want to think about specifically supporting audio controls as well as looping behavior (in terms of key controls)

I don't know if I'd call it a quick fix. There are plenty of attributes, like muted, loop, poster, etc that a lib consumer might want. But 100% agree that if people start doing weird stuff, we should take that as a sign that something should change.

I don't intend to add support in VIAME-Web client.

@subdavis subdavis merged commit 0e7c6fd into master Nov 11, 2020
BryonLewis pushed a commit that referenced this pull request Nov 16, 2020
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)

Add dummy generators

Very minor fixes
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
@subdavis subdavis deleted the client/mute branch November 30, 2020 19:58
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