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

Fixes for serverless HLS #646

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

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.

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.

1 participant