Closed 302Gerben closed 10 months ago
welcome[bot]
commented
1 year ago 💖 Thanks for opening this pull request! 💖
Things that will help get your PR across the finish line:
npm run lint -- --errors locally to catch formatting errors earlier.We get a lot of pull requests on this repo, so please be patient and we will get back to you as soon as we can.
adrums86
commented
1 year ago Thanks for submitting this PR and linking the issue! I read through everything and think I understand what the issue is, this fix may be preventing a problematic seek, but I'm not sure it's doing it in the way that's expected in the original issue. The seekable range is a TimeRanges object that will return a start and end, not correlating directly to the number of segments that are currently buffered. If the seekable ranges length is > 1 it essentially means there are 2 ranges that are seekable, for example if there's a buffer gap.
That said, I do think this needs to be addressed ASAP. I'm wondering if we can just remove the entire block of code that causes this issue? Per the TODO at the top of the function the scenario where a seek is before the seekable range or into a gap during live playback seems to be handled with the buffered checks below.
amtins
commented
1 year ago I may be totally off topic, as different comments in different issues seem to mention different problems.
@adrums86 I went through the different issues and it reminded me of https://github.com/videojs/mux.js/pull/430 because of https://github.com/videojs/http-streaming/issues/1049#issuecomment-780649483.
I tried to make a fix at the mux level that is based on the same logic but in m2ts ElementaryStream https://github.com/amtins/mux.js/pull/1.
It seems to fix the issue (but which one? 😅), the stream used to test is https://snapstream-dev-test-public.s3.us-east-1.amazonaws.com/videojs-pts-rollover-issue/playlist.m3u8
How to test
After https://amtins.github.io/http-streaming-mux/
However, if we take the stream, which is unavailable, the player continually tries each playlist. This seems to be expected behavior, as the player waits for a child playlist to become available again.
video-archivist-bot
commented
1 year ago Hey! We've detected some video files in a comment on this issue. If you'd like to permanently archive these videos and tie them to this project, a maintainer of the project can reply to this issue with the following commands:
@video-archivist-bot save KEPJ2w@video-archivist-bot save ZEnaQA@video-archivist-bot save Mj4M6wadrums86
commented
1 year ago Great catch @amtins ! Lets get that PR merged this week and update VHS with a new mux and see if this alleviates the issue for everyone.
amtins
commented
1 year ago @adrums86 I'll try to do a proper PR tomorrow, but maybe I could use a little help with the unit tests. However, it seems odd that I should have done this, as the various piplines call TimestampRolloverStream
adrums86
commented
1 year ago This might be fixed in #1433 can anyone confirm?
GeppieNL
commented
1 year ago This might be fixed in #1433 can anyone confirm?
We can confirm it is fixed by 1433! :tada:
adrums86
commented
10 months ago @GeppieNL Thanks for the confirmation. Closing this as it appears this is fixed in #1433.
Description
As stated in the following discussion, videojs player requests in certain situations indefinitely for the same chunks which results in a ddos being fired at the server. This issue keeps getting ignored/closed automatically.
https://github.com/videojs/http-streaming/issues/1000#issuecomment-899934566
This situation happens when there is a network congestion or just really slow internet speeds. (Chrome dev tools, throttling slow 3G for example) the player starts buffering after a few seconds. When the network recovers to normal speed, the player keeps requesting the same chunk over and over. (Imagine you have a hundreds of visitors..)
Specific Changes proposed
Check if there are seekable points.
Requirements Checklist