Skip to content

Fixes for serverless HLS#646

Open
marwyg wants to merge 3 commits intoopencast:masterfrom
marwyg:263-hls-fix
Open

Fixes for serverless HLS#646
marwyg wants to merge 3 commits intoopencast:masterfrom
marwyg:263-hls-fix

Conversation

@marwyg
Copy link
Member

@marwyg marwyg commented Jan 24, 2025

I made my own PR, I hope that's okay. The original draft is here: #627
This PR contains the actual changes from @luniki + changes after review from @JulianKniephoff. The review is also to be found here: #627.

The 3 points Julian made:

  1. Integrating the sort into the sorting below. I did that and it works. (Fixes for serverless HLS #627 (comment))
  2. Is it possible to remove renderers: ["html5", "native_hls"]completly?: Yes, I removed it and it works. (Fixes for serverless HLS #627 (comment))
  3. I didn't see a case where the duration was infinity, but that doesn't mean that Marcus didn't see something. I testet a small javascript function to see, how !isFinite and isNaN differs from each other:
function compareIsNaNAndIsFinite() {
  const testValues = [NaN, "hello", undefined, "123", 123, Infinity, -Infinity, null, true, false, ""];

  console.log("Value\t\tisNaN(value)\t!isFinite(value)");
  console.log("------------------------------------------------");
  testValues.forEach(value => {
    console.log(
      `${String(value).padEnd(10)}\t${isNaN(value)}\t\t${!isFinite(value)}`
    );
  });
}

compareIsNaNAndIsFinite();

The result:

Value		isNaN(value)	!isFinite(value)
------------------------------------------------
NaN       	true		true
hello     	true		true
undefined 	true		true
123       	false		false
123       	false		false
Infinity  	false		true
-Infinity 	false		true
null      	false		false
true      	false		false
false     	false		false
          	false		false

So I think, it's okay to use !isFinite() instead, because it can handle one more case, but behaves the same in the other cases.

I added another small line from Marcus, where he triggers an update for the video duration. It was reported, that the time wasn't displayed correctly.

@marwyg marwyg mentioned this pull request Jan 24, 2025
@marwyg
Copy link
Member Author

marwyg commented Jan 27, 2025

I tested this with the master and the stable branch.

@JulianKniephoff
Copy link
Member

I added another small line from Marcus, where he triggers an update for the video duration. It was reported, that the time wasn't displayed correctly

@marwyg do you have a reference for that? Like an issue or something? 🤔

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

Comments