Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: expose more of vrs methods #10

Merged
merged 2 commits into from
Aug 7, 2017
Merged

feat: expose more of vrs methods #10

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
59b4c43
feat: expose more of vrs methods
brandonocasey Aug 4, 2017
50d4185
make default private
brandonocasey Aug 4, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@
<script>
(function(window, videojs) {
var player = window.player = videojs('videojs-vr-player');
player.vr({projection: '360', debug: true});
player.mediainfo = player.mediainfo || {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

switched the demo to mediainfo as that is probably the best way to do it (unless you only have one video)

Copy link
Member

Choose a reason for hiding this comment

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

I don't love the way mediainfo has leaked out of the Brightcove Player and into a couple of our open source projects. Is there a different/better way?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We may be able to read spacial metadata from the video file. I am not sure that would work for HLS or if we even have access to it for mp4

Copy link
Member

Choose a reason for hiding this comment

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

Yeah. Ignore me, I don't think it's a big deal. It's not BC-specific.

Copy link
Member

Choose a reason for hiding this comment

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

It wasn't a leak, it was a conscious decision. At least for ads.

Copy link
Member

Choose a reason for hiding this comment

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

However, it seems to me like it might be possible to get rid of the mediainfo usage? Just do everything through the VR options and also expose it via player.vr.projection? Then update usage of current mediainfo to reflect that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we are dealing with switching media (I.E 360 video switches to non-360 or vice versa), it is not possible to tell wether a video is 360 or not. We could manually set it with player.vr.projection but it seems like it would be better to have a flag in the media itself that tells the player that the source is 360.

player.mediainfo.projection = '360';

// AUTO is the default and looks at mediainfo
player.vr({projection: 'AUTO', debug: true});
}(window, window.videojs));
</script>
</body>
Expand Down
Loading