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

Streams not playing on IE/Edge #1526

Closed
5 tasks done
Thomasvdam opened this issue Aug 2, 2016 · 24 comments · Fixed by #1548
Closed
5 tasks done

Streams not playing on IE/Edge #1526

Thomasvdam opened this issue Aug 2, 2016 · 24 comments · Fixed by #1548
Labels
Milestone

Comments

@Thomasvdam
Copy link

Environment
Steps to reproduce
  1. When trying to load streams in IE and Edge Dashjs with the development console closed! It happens around 9/10 times for us.
  2. We use the following snippet to log to a textarea since we can't open the console:
var text = document.createElement('textarea');
text.style.width = '1000px';
text.style.height = '1000px';

if (document.readyState === 'complete') {
    document.body.insertBefore(text, document.body.firstChild);
} else {
    document.onreadystatechange = () => {
        document.body.insertBefore(text, document.body.firstChild);
    };
}

function logToText(...args) {
    var result = '';
    for (var i = 0; i < args.length; i++) {
        var thing = args[i];

        result = result + JSON.stringify(thing) + '\n';
    }

    result = result + '------------\n';

    text.value += result;
}

console.error = logToText.bind(console);
console.log = logToText.bind(console);
console.warn = logToText.bind(console);
console.info = logToText.bind(console);
Observed behaviour

It seems to crash while loading the manifest. For some magical reason this issue does not occur when the development console is open, and I've also seen it work normally once in a while without the dev console.

Console output
"[5] Playback Initialized "
"[10] [dash.js 2.2.0] MediaPlayer has been initialized "

And then we get an error event from Dash.MediaPlayer.events.ERROR, which logs this

@LloydW93
Copy link
Member

LloydW93 commented Aug 2, 2016

Hey, can you please retest against #1527 - this adds a new "event" property to the error that includes the actual exception thrown and may help with identifying the problem.

@Thomasvdam
Copy link
Author

Thanks for your quick reply.

I was in a bit of a hurry yesterday and forgot to post the things we had already tried. We did something similar as in #1527, but just logged the error to the console. All we got was an emtpy object {}. So after some additional logging we found that the xml2json throws the empty error at this line in one of the last iterations (at least, we suspect it is one of the last iterations).
Updating it to the latest version seems to fix the issue but then we get a new exception in objectIron.run(), which we have not yet investigated further.
If I have time today I'll look into it further and report back with any additional findings.

@davemevans
Copy link
Contributor

The actual exception is TypeError: Unable to get property '_test' of undefined or null reference. This is being thrown by BaseMatcher when retrieving the derived Matchers test method.

It seems to vary when during the parsing this happens with no real pattern, but I note that this is a long presentation with an extremely large SegmentTimeline which will take a long time to process.

I wonder if Edge is incorrectly garbage collecting something and the garbage collection is less aggressive when the devtools are open.

I don't really know how to proceed with this.

@davemevans
Copy link
Contributor

Updating it to the latest version seems to fix the issue but then we get a new exception in objectIron.run(), which we have not yet investigated further.

Our xml2json has been modified specifically to add these matchers which do some type conversion. Adding them to the latest would take effort and probably suffer from the same problem. There is an open issue about upgrading at #1267.

@sandersaares
Copy link
Member

sandersaares commented Aug 4, 2016

I believe the Axinom maintained fork of xml2json has support for those matchers. We made it when there was no existing repo on GitHub and just copied in all the extra features from dash.js on top of the latest official code, I believe.

Likely it has diverged from any other "latest" version on GitHub by now.

See https://github.com/Axinom/x2js

@Thomasvdam
Copy link
Author

We're investigating this issue ourselves and were able to reproduce the issue in the dash reference player.

After loading the page in Microsoft Edge with the debug console closed (important) we select the first VOD, Sample Streams > VOD > SegmentTemplate/Number, live profile. This plays as expected. Then we select the second stream, Sample Streams > VOD > SegmentBase, ondemand profile, which does not play.
Repeating the above steps with the debug console opened will successfully play the second stream.

Hopefully this helps in pinpointing the issue.

@davemevans
Copy link
Contributor

@wilaw do I recall Kilroy offer to get someone to look at this? There's a reasonable amount of information in this ticket and it is reproducible.

@Doekie
Copy link

Doekie commented Aug 18, 2016

This issue is blocking us from going live with our player. Please advice on an ETA of a fix.

@NickDrouin
Copy link

NickDrouin commented Aug 19, 2016

While I can't help with the dashjs issues surrounding parsing the manifest, it should be known that the the Edge/Win10 Type 1 implementation (using a simple

Today, I am getting a 400 error on that link provided in the bug above:
http://manifest.us.rtl.nl/0/v175/none/dash/components/videorecorder/34/345829/345830/d8c5cb16-5c29-4afd-b8ec-fab83477c72b.ssm/Manifest.mpd
Can we get the link back up?

@NickDrouin
Copy link

Looking at the logged manifest from here:
https://gist.github.com/Thomasvdam/488a7f2f04455574eb591ebc0c3eaa31

I am seeing two video adaptation sets, the second of which has a different 'par' value for bitrate 183000. The first adaptation set has 7 representation that will adapt normally, the other has a single representation of 183kbps at 96x176 (par="11:6").

How are you expecting a player to handle this content?

Is this dual-video-adaptation-set intended, or an artifact of a unintended encoder settings?

@Thomasvdam
Copy link
Author

I'm afraid that particular link has expired and we're unable to set up a new one. Our problem originated from a different manifest that we couldn't share, and since that manifest showed the same problem we used that one instead. In hindsight this was a bad idea, but luckily we found an easy way to reproduce the issue in the reference player with the streams provided there.

I can't really help with strange video adaptation as we just dug it up from the web in the search for a stream that showed the same issues, sorry.

@bwidtmann
Copy link
Contributor

I can reproduce it in 2.1.1. I followed the advise in #1545 and downgraded to 2.1.0 and this fixed the problem for me.

@dsparacio
Copy link
Contributor

So we either need to revert #1396 which is a bandaid to the real issue that we believe is a bug in MS browser. I am not certain if it is a bug. The main questions is, How can a set var become null with nothing else setting it. Premature Garbage cleanup? @NickDrouin people report this issue does not appear when the Dev tools are open. Does the dev tools being open change the GC routine in any way?

One other options is to refactor the code a bit to see if maybe there is just an issue in the transpile that IE/Edge does not like. I will try to play with that today.

@dsparacio
Copy link
Contributor

If we do not have a clear answer in the next few days i will revert #1396 until we can figure this out.

@dsparacio
Copy link
Contributor

I am testing this on Windows 10 IE/Edge latest versions with VOD segment base and while I did see a failure with first play, it never failed again. I did open the console to get it to play. I clear cache and tried again with console closed and played without issue. this is the latest dev branch 2.3. Is this consistent to what others are seeing with latest? I think we need a better media example that fails every time before we go and revert #1396. This seems to be the challenge, getting failing media.

@bwidtmann
Copy link
Contributor

Hi @AkamaiDASH

I checked latest dev branch 2.3 as well and i could only start our content in Edge when the dev console was open, as stated in the issue above. Here is our test content:

manifestUrl: http://damxduspon100-s.akamaihd.net/scriptumtest/piff-production/content/test/testcontent/scriptum_free_test/scriptum_free_test_enc_720.ism/.mpd

licenseUrl: http://prosieben.live.ott.irdeto.com/playready/rightsmanager.asmx?CrmId=prosieben&AccountId=prosieben&ContentId=scriptum_free_test&SubContentType=Default

could you please re-check it on your side with our content? FYI: Maybe you have to wait a while when starting it as it is no productive content and therefore not on the CDN edges.

@bwidtmann
Copy link
Contributor

I think I have found a solution for the problem on Edge. @bbcrddave can you please check my PR #1548

I wonder why it worked with the old invoking of the function at all as you changed it to a real es6 class.

@LloydW93
Copy link
Member

Fyi, @bbcrddave is on leave this week so isn't around to check.

@davemevans
Copy link
Contributor

I believe invoking the function like that is still legit with an es6 class?

It's great if this solution works, but it'd be even better if someone explained how!

@Thomasvdam
Copy link
Author

Using the latest development branch and the fix provided by @bwidtmann we're able to switch between streams in Edge. Great find!

@bwidtmann
Copy link
Contributor

bwidtmann commented Aug 23, 2016

you should only use 'call' or 'apply' when you want to pass a different 'this' value to the function. in your new function you are relying on 'this' context of your es6 class instance. you should not change it.

why the old way works with opened browser console could be explained by the strict mode. in strict mode the passed 'this' object behaves differently:
In function calls like f(), the this value was the global object. In strict mode, it is now undefined. When a function was called with call or apply, if the value was a primitive value, this one was boxed into an object (or the global object for undefined and null). In strict mode, the value is passed directly without conversion or replacement.

@davemevans
Copy link
Contributor

Interesting. I suppose it's possible all the other major browsers fail to implement this correctly.

Doesn't explain why it doesn't fail immediately on the first call though, and sometimes doesn't fail.

Still, great we have a fix!

@dsparacio
Copy link
Contributor

@bwidtmann great find thanks for the confirmation it solves the problem @Thomasvdam - Merging the PR now.

@dsparacio dsparacio added this to the v2.3.0 milestone Aug 23, 2016
@dsparacio dsparacio added the Bug label Aug 23, 2016
@davemevans
Copy link
Contributor

davemevans commented Sep 1, 2016

in your new function you are relying on 'this' context of your es6 class instance. you should not change it.

This is incorrect. The function that is failing to resolve this is matchobj.test (this is an ES6 getter and therefore a function rather than an attribute). However, no modified this is passed to that function (the this in this instance should be the derived matcher instance which is set during construction of the class).

The function that is called with the modified this is what is returned by matchobj.test. The value of this is not used in the matcher (it never was), so it doesn't matter that it's undefined.

So I still believe the existing method should have worked, and there is a bug in IE/Edge (or the transpiling, but this seems very unlikely given this works in all other browsers and some of the time in IE/Edge). We will need to be careful when using (transpiled) ES6 getter/setter methods.

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

Successfully merging a pull request may close this issue.

8 participants