{"action":"created","issue":{"url":"https://api.github.com/repos/videojs/videojs-contrib-hls/issues/1416","repository_url":"https://api.github.com/repos/videojs/videojs-contrib-hls","labels_url":"https://api.github.com/repos/videojs/videojs-contrib-hls/issues/1416/labels{/name}","comments_url":"https://api.github.com/repos/videojs/videojs-contrib-hls/issues/1416/comments","events_url":"https://api.github.com/repos/videojs/videojs-contrib-hls/issues/1416/events","html_url":"https://github.com/videojs/videojs-contrib-hls/pull/1416","id":320984016,"node_id":"MDExOlB1bGxSZXF1ZXN0MTg2NDg0NDE0","number":1416,"title":"handle dynamic buffer sizes (4k support)","user":{"login":"squarebracket","id":127774,"node_id":"MDQ6VXNlcjEyNzc3NA==","avatar_url":"https://avatars0.githubusercontent.com/u/127774?v=4","gravatar_id":"","url":"https://api.github.com/users/squarebracket","html_url":"https://github.com/squarebracket","followers_url":"https://api.github.com/users/squarebracket/followers","following_url":"https://api.github.com/users/squarebracket/following{/other_user}","gists_url":"https://api.github.com/users/squarebracket/gists{/gist_id}","starred_url":"https://api.github.com/users/squarebracket/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/squarebracket/subscriptions","organizations_url":"https://api.github.com/users/squarebracket/orgs","repos_url":"https://api.github.com/users/squarebracket/repos","events_url":"https://api.github.com/users/squarebracket/events{/privacy}","received_events_url":"https://api.github.com/users/squarebracket/received_events","type":"User","site_admin":false},"labels":[{"id":601671396,"node_id":"MDU6TGFiZWw2MDE2NzEzOTY=","url":"https://api.github.com/repos/videojs/videojs-contrib-hls/labels/in%20progress","name":"in progress","color":"ededed","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":8,"created_at":"2018-05-07T22:45:00Z","updated_at":"2018-07-11T18:58:21Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/videojs/videojs-contrib-hls/pulls/1416","html_url":"https://github.com/videojs/videojs-contrib-hls/pull/1416","diff_url":"https://github.com/videojs/videojs-contrib-hls/pull/1416.diff","patch_url":"https://github.com/videojs/videojs-contrib-hls/pull/1416.patch"},"body":"## Description\r\nThis is a PR to allow for streams with large segment sizes, such as with 4K media. When a `QuotaExceededError` happens, the forward and back buffer are dynamically resized, and we keep track of how much data is in the buffer. We then use these as checks within the logic for determining if a segment should be downloaded, or if we should just chill. \r\n\r\nNote that this PR doesn't use any GOP info for determining what's safe or not. The reason is that we'd have to parse out the GOP info for fMP4 streams. I figure the best time to add that functionality is in VHS, where a) media-sources is part of the code and b) we'll have to parse fMP4 stuff anyway for things like captions. So that's planned for when this is ported to VHS.\r\n\r\nRequires the code in videojs/videojs-contrib-media-sources#178 to work.\r\n\r\n## Specific Changes proposed\r\n- Add `BACK_BUFFER_LENGTH` config\r\n- Make `goalBufferLength`, `maxGoalBufferLength`, and `backBufferLength` getters+setters on the master playlist controller, and pass them through to the segment loaders\r\n- Set the aforementioned values when we get a `QuotaExceededError` based on the amount of time and data in the buffers when it occurs\r\n- Make `safeBackBufferTrimTime` take a `backBufferLength` argument to use instead of the default 30s\r\n- Calculate a `nextSegmentSize` in the segment loader to determine if we might bust the buffer, either based on the current playlist's `BANDWIDTH` attribute if it exists or a running average of segment sizes if not\r\n- `checkBuffer_` checks to see if we might bust the buffer, and trims the back buffer and returns `null` if that's the case\r\n- Only remove data from a buffer if we can remove a whole segment (to keep estimates of data in buffer accurate)\r\n- Slice up the segment into 5MB chunks if we get a `QuotaExceededError` on the first append\r\n- Wait until we can append the segment if we get a `QuotaExceededError` after the first append\r\n- Add functions for calculating the min and max bytes in the buffer based on segment metadata cues (playhead is rarely at a segment boundary so we need both min and max functions for maximum safety)\r\n- Add function for removing non-buffered segment metadata cues\r\n- In the `SourceUpdater`, when we get a `QuotaExceededError`, move the `pendingCallback` to another variable (`deferredCallback`) and null out `pendingCallback` so that we can process a `remove()` and try to reappend the segment that caused the error\r\n\r\n## Requirements Checklist\r\n- [x] Feature implemented / Bug fixed\r\n- [ ] If necessary, more likely in a feature request than a bug fix\r\n  - [x] Unit Tests updated or fixed\r\n  - [ ] Docs/guides updated (no user facing changes)\r\n  - [ ] Example created ([starter template on JSBin](http://jsbin.com/liwecukasi/edit?html,output))\r\n- [ ] Reviewed by Two Core Contributors\r\n"},"comment":{"url":"https://api.github.com/repos/videojs/videojs-contrib-hls/issues/comments/404275514","html_url":"https://github.com/videojs/videojs-contrib-hls/pull/1416#issuecomment-404275514","issue_url":"https://api.github.com/repos/videojs/videojs-contrib-hls/issues/1416","id":404275514,"node_id":"MDEyOklzc3VlQ29tbWVudDQwNDI3NTUxNA==","user":{"login":"ldayananda","id":3826638,"node_id":"MDQ6VXNlcjM4MjY2Mzg=","avatar_url":"https://avatars2.githubusercontent.com/u/3826638?v=4","gravatar_id":"","url":"https://api.github.com/users/ldayananda","html_url":"https://github.com/ldayananda","followers_url":"https://api.github.com/users/ldayananda/followers","following_url":"https://api.github.com/users/ldayananda/following{/other_user}","gists_url":"https://api.github.com/users/ldayananda/gists{/gist_id}","starred_url":"https://api.github.com/users/ldayananda/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ldayananda/subscriptions","organizations_url":"https://api.github.com/users/ldayananda/orgs","repos_url":"https://api.github.com/users/ldayananda/repos","events_url":"https://api.github.com/users/ldayananda/events{/privacy}","received_events_url":"https://api.github.com/users/ldayananda/received_events","type":"User","site_admin":false},"created_at":"2018-07-11T18:58:21Z","updated_at":"2018-07-11T18:58:21Z","author_association":"NONE","body":"These are just initial questions to help me understand the chosen strategy a little better:\r\n\r\n>Make goalBufferLength, maxGoalBufferLength, and backBufferLength getters+setters on the master playlist controller, and pass them through to the segment loaders\r\n\r\nRather than have this be getters/setters on the master playlist controller, these could just be settings/options passed into the master playlist controller, then passed down to and modified on the segment loaders. \r\n\r\n>Set the aforementioned values when we get a QuotaExceededError based on the amount of time and data in the buffers when it occurs\r\n\r\nWhy does the time matter if we are also using the amount of data in the buffer?\r\n\r\n>checkBuffer_ checks to see if we might bust the buffer, and trims the back buffer and returns null if that's the case\r\n\r\nSeems like trimming the back buffer should be done in another method outside `checkBuffer_`\r\n\r\n>Slice up the segment into 5MB chunks if we get a QuotaExceededError on the first append\r\n\r\nIf I understood correctly, we are appending and removing whole segments in this PR right? Why is it needed to split up the segment into chunks?\r\n\r\n>Add functions for calculating the min and max bytes in the buffer based on segment metadata cues (playhead is rarely at a segment boundary so we need both min and max functions for maximum safety)\r\n\r\nIs there a specific use for the *min* bytes in the buffer? Would the ideal be to play it safe and always assume the max is in the buffer?\r\n\r\n>Add function for removing non-buffered segment metadata cues\r\n\r\nI thought this exists in [contrib-media-sources](https://github.com/videojs/videojs-contrib-media-sources/blob/master/src/remove-cues-from-track.js)?\r\n\r\n>In the SourceUpdater, when we get a QuotaExceededError, move the pendingCallback to another variable (deferredCallback) and null out pendingCallback so that we can process a remove() and try to reappend the segment that caused the error\r\n\r\nReading this naively, it seems like the remove and reappend attempt can be done before the original `pendingCallback` is run. Is there something preventing this?"}}
{"actor":{"display_login":"ldayananda"}}