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.

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

add HLS MacOS support and HLS example, per https://github.com/aframevr/aframe/pull/2657#issuecomment-312349199 #2830

Closed
wants to merge 9 commits into from

Conversation

machenmusik
Copy link
Contributor

@machenmusik machenmusik commented Jun 30, 2017

add HLS example, converted from #1846 (comment)

@machenmusik
Copy link
Contributor Author

tested using native HLS support in Edge browser, using local server run as npm start

@Utopiah
Copy link
Contributor

Utopiah commented Jun 30, 2017

For other browsers https://github.com/video-dev/hls.js

@dmarcos
Copy link
Member

dmarcos commented Jul 2, 2017

The example is a bit buried. Wouldn it be better to add it to the existing videosphere example? We could add a button on screen to switch to the hls one. What do you think?

@machenmusik
Copy link
Contributor Author

I thought about something like that but was unsure since many browsers do not support native HLS.

@dmarcos
Copy link
Member

dmarcos commented Jul 2, 2017

We could make the existing example the default one and a button on the corner to switch to the HLS one

@machenmusik
Copy link
Contributor Author

@dmarcos you mean like the latest commit? CSS styling welcome...

</head>
<body>
<div style="right:8px; top:8px; position:absolute; z-index:999999">
<a style="align:right" href="../../test/videosphere/hls.html">Click here for<br/>native HLS version</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

align:right? do you mean text-align: right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps - though looked ok as-is... could use nicer styling?

Copy link
Contributor

Choose a reason for hiding this comment

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

I’d remove the line break

@machenmusik
Copy link
Contributor Author

This one has gotten a little stale - do we still want it? If so, are any further styling changes needed (and if so, what?)

@@ -5,8 +5,12 @@
<title>360 Video</title>
<meta name="description" content="360 Video — A-Frame">
<script src="../../../dist/aframe-master.js"></script>
<link rel="stylesheet" href="/css/examples.css">
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make this path relative?

</head>
<body>
<div style="right:8px; top:8px; position:absolute; z-index:999999">
Copy link
Contributor

Choose a reason for hiding this comment

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

can you move all these inline styles to be encapsulated in a <style> block?

</head>
<body>
<div style="right:8px; top:8px; position:absolute; z-index:999999">
<a style="align:right" href="../../test/videosphere/hls.html">Click here for<br/>native HLS version</a>
Copy link
Contributor

Choose a reason for hiding this comment

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

I’d remove the line break

<a-assets>
<video id="city" preload="true"
src="../../assets/video/city_halfres_hls.m3u8"
width="360" height="360" autoplay loop="true"
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove the value from the loop attribute. its presence means truthy.

<video id="city" preload="true"
src="../../assets/video/city_halfres_hls.m3u8"
width="360" height="360" autoplay loop="true"
crossOrigin="anonymous"></video>
Copy link
Contributor

Choose a reason for hiding this comment

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

you can lowercase the O

@machenmusik
Copy link
Contributor Author

Revisiting this, it turns out that changes were needed to make MacOS HLS work... and iOS HLS isn't working again :-/

@@ -66,6 +66,11 @@ function isIOS () {
}
module.exports.isIOS = isIOS;

function isMacOS () {
return /Mac/.test(window.navigator.platform);
Copy link
Contributor

Choose a reason for hiding this comment

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

I would check /Mac/i just in case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems to me that actually increases the risk of false detection, Apple / Safari is pretty consistent about correct case for Mac and iDevices

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you feel strongly about this one? I was actually wondering if it would be better to look for Mac OS in navigator.userAgent than to check against values like MacIntel in navigator.platform

Copy link
Contributor

Choose a reason for hiding this comment

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

you can leave this. that's a good point - hadn't thought of that.

@@ -49,6 +50,7 @@ module.exports.AScene = registerElement('a-scene', {
createdCallback: {
value: function () {
this.isIOS = isIOS;
this.isMacOS = isMacOS;
Copy link
Contributor

Choose a reason for hiding this comment

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

do you want to create a this.isApple?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't know what other Apple products like tvOS may do... if we have to go beyond just the two variants, then maybe.

Copy link
Contributor

Choose a reason for hiding this comment

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

Per Apple's Streaming page:

Send live and on‐demand audio and video to iPhone, iPad, Mac, Apple TV, and PC with HTTP Live Streaming (HLS) technology from Apple.

@@ -49,6 +50,7 @@ module.exports.AScene = registerElement('a-scene', {
createdCallback: {
value: function () {
this.isIOS = isIOS;
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be handled in a separate commit, but what do you think about consolidating these booleans to a single this.supports = {ios: true, mac: true} object property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, the semantics are less supports then is... I do think the map form will prevent the number of is permutations down.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should do this one later, there are a bunch of helper functions this might be good for but may want to discuss the approach a bit more

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, that's fine - if it's changed, will have to support both properties for a while and throw deprecation Console warnings until the old properties are deprecated.

feel free to file an issue if you think it warrants one right now. if you don't think it's a priority, we can punt on filing for this.

@machenmusik
Copy link
Contributor Author

Does anyone have a non-upgraded iOS 10 device to try, to confirm that iOS HLS behavior regressed between 10 and 11?

@cvan
Copy link
Contributor

cvan commented Oct 12, 2017

@machenmusik: I have a device (for just this purpose!)

I’ll test it tomorrow and get back to you.

thanks for taking this on :)

@machenmusik
Copy link
Contributor Author

Great, thanks @cvan - I can't downgrade back to 10.3.3 anymore so that will be super helpful

Copy link
Contributor

@cvan cvan left a comment

Choose a reason for hiding this comment

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

Phew, so there's a lot going on. Lots of small things – HLS playback across macOS desktop and iOS mobile is pretty tricky. Kudos for taking this on! It's a good addition to suport. iOS video playback is a very popular use case of A-Frame.

So, after my various testing…

This video plays properly on my iPhone 5 running iOS 10.3.2 (Safari/602.1), but the video fails to be displayed on my iPhone 7 running iOS 11.0.2 (Safari/604.1).

See my comments for my hypotheses and misc. insights.

Let me know your thoughts on this, and I'll give this all another follow-up review. Thanks for your patience and for the PR - I really appreciate it.

</a-entity>
<a-text value="Click or tap to play." align="center" position="0 0.25 -2"></a-text>
<script>
setTimeout(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you change this to window.addEventListener('load', function () {?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

<a-text value="Click or tap to play." align="center" position="0 0.25 -2"></a-text>
<script>
setTimeout(function() {
var vid = document.querySelector('video');
Copy link
Contributor

Choose a reason for hiding this comment

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

can you check vid.addEventListener('load', function () { … }) instead of assuming it's been loaded in 3 seconds?

Copy link
Contributor

Choose a reason for hiding this comment

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

also, see my comment below about the 3000-millisecond timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

vid.addEventListener did not work, BTW

<a-entity material="shader: flat; src: #city" geometry="primitive: sphere; radius: 100"
scale="1 1 -1">
</a-entity>
<a-text value="Click or tap to play." align="center" position="0 0.25 -2"></a-text>
Copy link
Contributor

Choose a reason for hiding this comment

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

can you change this to read Click to play video.?

Copy link
Contributor

Choose a reason for hiding this comment

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

it's unfortunate that this will be in English only. localised text and/or a clear visual indicator would be ideal, but I realise that's most certainly outside the scope of this PR 😜

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, let's get the video playing as well as possible first, and then we can try to change to something universally recognizable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed - I don’t want to distract you with enhancements that will prevent you from landing this soon. I just wanted to bring it up - hope that’s all right.

</head>
<body>
<a id="hls" href="../../test/videosphere/hls.html">
Click here for native HLS version
</a>
<a-scene>
<a-assets>
<video id="video" src="https://ucarecdn.com/fadab25d-0b3a-45f7-8ef5-85318e92a261/"
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add webkit-playsinline playsinline here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -0,0 +1,37 @@
<!DOCTYPE html>
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add this line to the Tests section of examples/index.html?

      <li><a href="test/videosphere/hls.html">Video <em>(HLS for iOS/macOS streaming)</em></a></li>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

take a look at the wording?

<a-assets>
<video id="city" preload="true" webkit-playsinline playsinline
src="../../assets/video/city_halfres_hls.m3u8"
width="360" height="360" autoplay loop
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the S in HLS stands for Streaming, so this video won't loop.

I'd recommend one of the two solutions:

  1. removing the loop attribute and adding an HTML comment about how HLS is for streaming content only
  2. adding some JS (only for this example) to the <script> below to auto-replay it:
          var LOOP_WAIT_TIME_MS = 20;
          var loopTimeout;

          vid.addEventListener('play', function () {
            if (!/\.m3u8$/i.test(vid.currentSrc)) { return; }
            loopTimeout = window.setTimeout(function () {
              loopTimeout = null;
              vid.pause();
              vid.play();
            }, (vid.duration - vid.currentTime) * 1000 + LOOP_WAIT_TIME_MS);
          });

          vid.addEventListener('pause', function () {
            if (!/\.m3u8$/i.test(vid.currentSrc)) { return; }
            if (loopTimeout) {
              clearTimeout(loopTimeout);
              loopTimeout = null;
            }
          });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually, my recollection is that it does loop (HLS is a container format, it's not prescriptive about the Streaming part) -- did it not for you? the video itself is only 8 seconds long

Copy link
Contributor

Choose a reason for hiding this comment

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

nope, only 8 seconds and stopped - with the loop attribute set

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that is odd. it loops for me, although it does pause slightly at the end before restarting (which I presume is Safari quirk)

<a-entity material="shader: flat; src: #city" geometry="primitive: sphere; radius: 100"
scale="1 1 -1">
</a-entity>
<a-text value="Click or tap to play." align="center" position="0 0.25 -2"></a-text>
Copy link
Contributor

Choose a reason for hiding this comment

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

this is quite easy to miss on mobile, especially after you enter VR mode and rotate the phone to a landscape orientation.

what are your thoughts about positioning this to the user's gaze (overriding the default <a-entity data-aframe-default-camera camera>)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

window.removeEventListener('click', playOnce);
});
}
}, 3000);
Copy link
Contributor

Choose a reason for hiding this comment

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

while this is the default for <a-assets>, it could be different if a timeout attribute is specified containing a different value. per my comment above, it'd be best to listen for the events. (if you can't, because of a WebKit bug, I'd recommend changing this setTimeout to a setInterval, and then check until vid has loaded.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

a-assets isn't doing anything, I removed it

Copy link
Contributor

Choose a reason for hiding this comment

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

it doesn’t prevent the loading, but it throws an uncaught Promise or doesn’t resolve (because the bufferedSeconds value is wrong - and because of that if statement I referenced in other comments).

so, yeah, should be fine - checking for vid.paused should be fine without waiting for some event. I didn’t test that too closely. confirmed on your end?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems to work - no prompt on Edge, prompts and plays properly when clicked on macOS Safari. I can't test on iOS 10.x as mentioned previously

<a-entity material="shader: flat; src: #city" geometry="primitive: sphere; radius: 100"
scale="1 1 -1">
</a-entity>
<a-text value="Click or tap to play." align="center" position="0 0.25 -2"></a-text>
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO, this should not be visible if the video could be successfully autoplayed (even if that's not technically possible due to the autoplay on-screen constraints in iOS 10)

Copy link
Contributor

Choose a reason for hiding this comment

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

also: see this related WebKit bug, which should hopefully help feature-detect with JS whether a video's autoplay is being restricted: https://bugs.webkit.org/show_bug.cgi?id=161804

#EXTINF:7.166667,
city_halfres_hls0.ts
#EXTINF:0.866667,
city_halfres_hls1.ts
Copy link
Contributor

Choose a reason for hiding this comment

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

does this indicate an audio track (albeit a possibly empty one)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Those aren't separate tracks, those are separate segments; if you look with VLC for example you can see there is no audio

@ngokevin
Copy link
Member

This seems like a pretty narrow use case (iOS + video + streaming + HLS). This seems like it could be done fairly easily outside of A-Frame without A-Frame supporting it specifically?

@cvan
Copy link
Contributor

cvan commented Oct 17, 2017

why limit A-Frame’s video capabilities? and, if A-Frame decides to explicitly not support these video use cases, provide a working solution for integration with A-Frame. if A-Frame doesn’t just work out of the box, potential big names will just move on to a different WebVR framework that does. Similar to what happened with Shopify VR prototyping with A-Frame but ended up settling on PlayCanvas.

@ngokevin
Copy link
Member

Because there's a maintenance cost, complexity, and it can be accomplished outside A-Frame. I believe A-Frame is successful with adoption in the WebVR space, I don't think it hinges on iOS streaming video support.

@machenmusik
Copy link
Contributor Author

(1) It's not just iOS. For example, it actually works better on Edge and WinMR because it has native support, full hardware acceleration, and can handle live. Same for Android, and especially Oculus Browser. iOS is actually the worst, since they broke it again with iOS 11.

(2) I don't think there is actually much complexity here, comparatively speaking - can you say more about what you are objecting to specifically, rather than a general less-is-more maintenance sentiment?

(3) Video is an important use case, there is quite a bit of it. HLS is still used on multiple platforms to deliver live and prerecorded content,

If you are saying you don't want A-frame to maintain the list of which things need which quirks, I understand. But having the support to be able to apply workarounds for the quirks is definitely something for core A-Frame. There is no other way to externally apply the quirks, because even if you set the shader manually, A-frame would set the wrong texture format for THREE to try.

@machenmusik
Copy link
Contributor Author

@Utopiah can you clarify, there is already support in A-Frame for native HLS (although it would appear that Apple broke it in iOS 11) -- is the ask for adaptive streaming regardless of native support (e.g. hls.js, dash.js, and the like) ? From prior discussions with @dmarcos @ngokevin, that level of support may be more appropriate for components at the moment.

@Utopiah
Copy link
Contributor

Utopiah commented Oct 24, 2017

@machenmusik yes adaptive streaming regardless of native support, be it within core or as a component (whatever is the most appropriate maintenance wise) there is a clear need for it from video professionals.

@machenmusik
Copy link
Contributor Author

thanks @Utopiah -- outside the various Apple/Safari breakage bugs (CORS, iOS 11) that is doable now actually...

@Utopiah
Copy link
Contributor

Utopiah commented Oct 24, 2017

@machenmusik yes I've done it in March and it worked relatively well back then but it was neither complete nor obvious.

@machenmusik
Copy link
Contributor Author

Just checking - am I right that the current PR no longer needs changes, but that there is still some discussion about removing the BGRA and flip Y functionality and/or quirk detection from core, with @dmarcos @ngokevin for removal, and @cvan @machenmusik @Utopiah against?

If so, can we merge this PR first so the current state is less broken / inconsistent, and open another issue to discuss how to refactor quirk detection etc. out of core?

@machenmusik
Copy link
Contributor Author

I would also like to point out that having a working HLS example is actually of rather significant benefit to those platforms that support native HLS without Safari's quirks, with the newest notable being Edge for WinMR.

@ngokevin
Copy link
Member

I'd support it in a component. It'd probably take you 20-30 minutes to publish, and the code path much, much easier to maintain. I'd keep the material component simple; I myself just make my own material component when it doesn't support something I need, much quicker than trying to bake/test it into core. If you need to update it, does not need to go through us. It'd be more discoverable as well as a component.

Regards to PR, I'd vote to close and then remove the quirk-hacking, and throw up a component.

@machenmusik
Copy link
Contributor Author

I agree that we want to move quirk detection and handling out of core as much as possible.

So IIRC the one thing that needs to be in core is selection of the THREE texture format (e.g. simple RGB vs. RGBA, which then gets interpreted as BGRA by shader in some cases). Video hardware accelerated decoding actually provides YUV colorspace and it is vastly more efficient to do color space conversion in shader if not intrinsic to the HW acceleration, and if WebGL can get that to us, performance can approach what can be done with native. Also, not sure if THREE hides, but on some platforms native video runs faster as 32 bit RGBA or ARGB...

So OK, I propose this:

  • keep this PR, modify it so it's just about having a working HLS example and ignoring any particular changes for quirks, then merge it. So it may not work on Safari, but should on Edge et al.

  • start working another issue/PR to remove all but the necessary texture format specification from the current quirk accommodations

  • in tandem with that PR, make an example component outside core that does the quirks

Agree?

@ngokevin
Copy link
Member

What do we need from the PR to have a working HLS example? We can't have an hls-video component that does everything start-to-finish with a Glitch that we can publish to the docs? I was proposing not only the quirk handling, but entire feature and handling altogether. You can create your own texture in the component as well, no need for A-Frame to flip/specify texture properties.

@machenmusik
Copy link
Contributor Author

I think the very simple case of adding, alongside the existing MP4 video demo, an equivalent that uses HLS - which was the original call for action here - has somehow gotten wrapped around the axle of Safari quirks and concern over whether they should be accommodated.

So let us please reset the discussion to be about simply adding a video example that uses HLS instead of MP4, with appropriate files and links from existing examples, and I will do the same to the contents of this PR. MP4 files are not suitable for live streaming cases, which is the space that support for HLS format and/or Media Source Extensions (MSE) covers nicely. I presume that if this is done, there will be no objection to merging the PR to provide the example, and we can discuss other concerns separately.

There are several prominent browsers that support native HLS video in A-Frame without any special considerations, which we should use to help politely suggest similar behavior from browsers with broken native HLS support. Note that browsers that support MSE have other paths to support live content, such as DASH and dash.js, so they may not benefit from adding HLS native support.

@dmarcos
Copy link
Member

dmarcos commented Oct 27, 2017

I'm also leaning to keep this as a separate component and let it evolve independently. There seems to be too many quirks and edge cases on a device and browser basis. Video streaming in general seems a bit of a moving target. If we feel there's a good paved way we can recommend I would reconsider to put it in the core.

Some additional info to consider when gauging demand of video in WebVR based on personal observation:

  1. For people starting without 3D graphics knowledge, video or panos seem the path of least resistance to start kicking the tyres and learn A-Frame. More often than not, the end goal of people doing video is not shipping video in production. Many of those questions we see about "How do I render Youtube, or this and that video?" come from people on a first exposure to WebVR / A-Frame and quickly move to other kind of experiences. A basic support of video for learners might be enough in the core. With this I'm not saying there is low demand for video content but something to consider is that the perception might be higher than the actual demand.
  2. About KRPano. It is a specialized product that supports a wide range of 360 image and video formats. It also comes along with tools to prepare assets. They provide an end to end workflow. I imagine a whole new separate project focused on 360 video / panos. A-Frame core should not aim at covering so much ground.

@machenmusik
Copy link
Contributor Author

machenmusik commented Oct 27, 2017

With all due respect, it seems clear that you guys are not highly fluent in the nuances of video support in browsers. Let me make a few statements and see whether we agree...

  • A-Frame supports video as a core media type, like 3D objects, and audio, and text (which took some convincing, but has proven its value over time.)

  • It does this because supporting experiences using (both traditional 2D and newer 180/360) video is a common use case for A-Frame, similar to panoramic 360 images and the various experiences built from and with them. (Also, new uses of video-as-2D texture mixed with 3D points / mesh are starting to emerge such as DepthKit.)

  • The HTML specifications and browser implementations are responsible tor video element and handling, not A-Frame (or three.js).

  • The details of video element support for various codecs and container formats (e.g. MP4, webm, HLS) are provided by HTML specifications and browser implementations, and are out of scope for three.js and A-Frame.

  • A-Frame introduces a very minimal layer, atop three.js at present, so that the material component can reference video elements much the same way they can reference image elements, and they can be visualized and utilized alongside other (three.js and) A-Frame functionality.

  • A-Frame should provide examples of how to use the minimal layer it provides for each core media type, or else it should drop support for the core media type. (NOTE: This is the intent of the current PR.)

  • A-Frame cannot guarantee, and should not attempt to guarantee, that said examples will work on all browser implementations, as that is out of scope. (This is the discussion we are having about quirks, and should be separated into other issues / PRs.)

Ok?

@machenmusik
Copy link
Contributor Author

Existing A-Frame video examples work when the browsers provide native support (e.d. Safari, Chrome, Edge, Firefox) but may not work in browsers that don't support that kind of video (if I recall correctly, that includes MP4 in Chromium, and webm in Safari at least).

The same should and will be true for these HLS examples once I strip down this PR.

We can refactor to simplify (by removing quirk special handling) separately.

Ok?

@ngokevin
Copy link
Member

Video is supported in-core minimally. Will detect if a video is passed and use VideoTexture if so. Any workarounds or additional needs supported outside core. HLS is especially even more of a niche demand. If KRPano offers something good in those regards, that's great. We'll continue to listen to opinions, but I don't think we can assuage anyone that disagrees fundamentally how we guide A-Frame's direction.

I hope this decision is understandable to some capacity since these conversations have taken too much time. Here's moving forward:

  • Closing the PR.
  • Removing any special HLS shaders and hooks and such.
  • Recommend anyone interested to create a component/Glitch to handle these needs. Have the component act outside the A-Frame material codepath. We can feature in the docs, there's no difference to interested developers where it lives. Measure how well it goes, then we re-evaluate in the future.

It should be extremely easy to release a component and/or Glitch. This is foundational to A-Frame's philosophy: permissionless innovation and extensibility. If anyone is passionate about helping with video, publish a component/Glitch/guide that will help people today; no one has stepped up there. It is a much more efficient path than trying to maintain in material component's dizzying code path.

@ngokevin ngokevin closed this Oct 28, 2017
@machenmusik
Copy link
Contributor Author

@ngokevin I think we agree on the direction, very specifically that A-Frame will continue to support video in core to the extent the underlying implementations expose it, but not include any customizations for deficiencies or quirks in behavior. In fact we expect to remove existing workarounds in subsequent PRs. Do I misunderstand?

On the fervent hope that we agree there... I really don't understand the objection to simply having an example of video playback with A-Frame that points at HLS files, to sit alongside the MP4 and webm examples currently distributed. It's like refusing to have examples of JPEG images, only PNGs... I am still in the air, but I will submit a new PR that has just the examples directory files from here, as I think this PR has confused matters by mixing concerns.

@ngokevin
Copy link
Member

ngokevin commented Oct 28, 2017

The prescribed way to add examples is to create a Glitch and link from the documentation. That way people can remix it/try it out instantly, and the code is more copyable since the dist is not a relative path to a Browserify build.

Hosting on the repo means we'd have to maintain it, any little changes need to get reviewed/tested, and indicates a level of quality of support which I can't guarantee.

@machenmusik
Copy link
Contributor Author

Ok good, so no objection to examples, but don't want to use the pre-existing examples folder methodology in the repo for reasons above. Got it, makes sense, thanks!

@machenmusik
Copy link
Contributor Author

So for the examples like the showcase videosphere which are currently examples folder in repo, do we want to move them out into one glitch with all the examples? Or individual glitches? Or just leave them alone?

@codechikbhoka
Copy link

codechikbhoka commented Oct 30, 2017

so what is the final decision ? If I want to play hls stream inside aframe, what is the recommended way ? currently I am using videojs library which wraps the video element inside the a-assets. It is working for chrome/firefox/safari on desktop, chrome on android but not on (safari/chrome) on iphone (iOS version 11.0)?

@machenmusik
Copy link
Contributor Author

currently I am using videojs library which wraps the video element inside the a-assets.

Assuming that means video.js with hls.js support, that is required for browsers that do not support HLS natively.

but not on (safari/chrome) on iphone (iOS version 11.0)?

Unfortunately there seems to be a Safari regression with iOS 11.

@codechikbhoka
Copy link

codechikbhoka commented Oct 30, 2017

Hey @machenmusik ,
Thanks for the reply.
So can we expect this to be solved in a few days ? How long could it take before this Safari regression with iOS 11 is taken care of. Is there a thread already open for this ?

@machenmusik
Copy link
Contributor Author

I believe this issue is best followed via mrdoob/three.js#9754, which has a long history.

@cvan
Copy link
Contributor

cvan commented Oct 30, 2017

and https://bugs.webkit.org/show_bug.cgi?id=163866#c17 is the WebKit bug to follow

@machenmusik
Copy link
Contributor Author

BTW, if more typical MP4 video is an option for you, that seems to work OK on Safari as long as you avoid Webkit/Safari CORS bugs and/or if Safari fixes for those bugs have landed in the version you use

@machenmusik
Copy link
Contributor Author

Thanks @cvan

@codechikbhoka
Copy link

yeah, mp4 is working fine on iphone safari iOS11. Thanks @cvan and @machenmusik for the webkit bug thread and three.js issue thread

@ngokevin
Copy link
Member

We can leave the examples folder as is for now, perhaps for testing core features and components, but start to move towards Glitches for examples and demonstrations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants