Conversation
KatrinIhler
force-pushed
the
263-hls-fixes
branch
from
9e54332 to
3869297
Compare
| RegExp.prototype.test.bind(/application\/.*|video\/.*/), | ||
| _.property("mimetype") | ||
| )); | ||
| videos = _.sortBy(videos, "master").reverse(); |
There was a problem hiding this comment.
Can we integrate this in the sort below? Something like
videos.sort(
util.lexicographical([
util.firstWith(
_.property("master")
),
// [...]
])
);Should work, but please double-check me on this.
There was a problem hiding this comment.
I integrated the sort as you suggested. It works: #646
| window.Hls = Hls; | ||
| mediaElementPlayer = new mejs.MediaElementPlayer(targetElement, { | ||
| renderers: ["html5", "native_hls"], | ||
| renderers: ["native_hls", "html5"], |
There was a problem hiding this comment.
Is this necessary even with the correct sort order of the videos? 🤔
Looking at the docs for this setting I also wonder: Can we get away with not specifying this at all?
There was a problem hiding this comment.
I removed the renderers line. I read the documentation and this field doesn't seem mandatory. It still works when I removed it: #646
| $(mediaElement).on("canplay durationchange", function () { | ||
| // If duration is still not valid | ||
| if (isNaN(self.getDuration()) || mediaElement.readyState < 1) { | ||
| if (!isFinite(self.getDuration()) || mediaElement.readyState < 1) { |
There was a problem hiding this comment.
Wouldn't live streams have infinite durations? 🤔 Although to be fair, the rest of the tool can't deal with those, either. x)
Still, I wonder: Did you encounter a case where the duration is infinite first, and then changes to something finite?
There was a problem hiding this comment.
As mentioned in #646, I didn't see such a case and I cannot say if Marcus did. But I compared in my other PR both functions (isNaN and !isFinite) and I think it should be okay like this.
|
Pinging @luniki to look at this review since he is the original author of the patch IIRC. |
PR opencast#627: Fixes for serverless HLS
|
I took the changes from here, addressed @JulianKniephoff review points and made my own PR #646. Maybe someone can take a look again, I think this should be okay now. |
|
Closing in favor of #646 |
3 participants
Actual changes from @luniki.