Optional offset param to IVideoSource.selectVideoTrack for partial buffer clear

[jonoward]

Mainly for use in a custom ABR manager, where calling StreamVideoSource.selectVideoTrack could pass an optional offset param, which is the number of seconds in front of video.currentTime that the buffer should be cleared from.

Useful when configuring the player with a large buffer that should be truncated when upgrading from a lower bitrate to a higher one (so that the user sees the newer bitrate without having to player through the size of the buffer)

Addresses issue #95

[tdrews]

80 char limit.

[jonoward]

Hi @tdrews, I've updated the branch with the suggested changes (have squashed the commits, not sure if you prefer that or not?), with the exception of removing the @expose annotations. Reason being is that without it, the compiler is renaming both the selectVideoTrack function and the videoSource variable (which is now just a protected member), which breaks an external custom ABR manager. Is there a better way to stop the compiler from renaming?

[tdrews]

Ah, yes, you are right about @expose. AFAIK the only other way to stop the compiler from renaming is to add @dict to SimpleAbrManager and then only reference videoSource via this['videoSource'], but I'd like to avoid that.

Does your custom ABR manager only need to access videoSource to call IVideoSource.selectVideoTrack() with the clear ahead param? Could SimpleAbrManager.selectVideoTrack() itself just take an optional clear ahead param, so videoSource can stay private (my apologies for backtracking on this)?

[jonoward]

Yes the only reason to expose videoSource was to call videoSource.selectVideoTrack with different parameters. No worries about the change, I've updated the PR so that videoSource remains private and SimpleAbrManager.selectVideoTrack has the exta parameter for the offset.

[shaka-bot]

Testing in progress...

[shaka-bot]

CI error: Failed to parse log file!

[tdrews]

:( Our bot is having a bad day. Code looks good, but there are some linter errors. Can you check with build/lint.sh ?

[jonoward]

The linter is returning some additional errors compared to the latest master branch (all "Missing space before.." or "Wrong indentation: expected any of.." errors) - should there be zero errors? I wasn't sure if this was a requirement as master also has these (about 200 errors). Unless me running this on Windows via Cygwin is giving different results? Here is a gist of the the linter log for this branch: https://gist.github.com/jonoward/a49b613c9ac5c988779c

[tdrews]

Hmm, that's strange. There should be 0 linter errors on master. Perhaps this is an issue with running the linter on Cygwin as you suggested; I'll add an issue to track this. The only linter errors I see are indentation ones,

----- FILE  :  source_buffer_manager.js -----
Line 315, E:0006: Wrong indentation: expected any of {2, 57} but got 4
Line 317, E:0006: Wrong indentation: expected any of {2, 57} but got 4
Line 318, E:0006: Wrong indentation: expected any of {2, 57} but got 4
Line 319, E:0006: Wrong indentation: expected any of {2, 57} but got 4
Line 320, E:0006: Wrong indentation: expected any of {4, 59} but got 8
Line 321, E:0006: Wrong indentation: expected any of {8, 16, 59, 63} but got 12
Line 322, E:0006: Wrong indentation: expected any of {4, 59} but got 8
Line 323, E:0006: Wrong indentation: expected any of {4, 59} but got 8
Line 324, E:0006: Wrong indentation: expected any of {2, 57} but got 4
Line 326, E:0006: Wrong indentation: expected any of {2, 57} but got 4
Line 328, E:0006: Wrong indentation: expected any of {2, 57} but got 4
Line 329, E:0006: Wrong indentation: expected any of {4, 59} but got 8
Line 330, E:0006: Wrong indentation: expected any of {4, 59} but got 8
Line 331, E:0006: Wrong indentation: expected any of {2, 57} but got 4
Line 333, E:0006: Wrong indentation: expected any of {2, 57} but got 4
Line 502, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 504, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 505, E:0006: Wrong indentation: expected any of {4, 60} but got 8
Line 506, E:0006: Wrong indentation: expected any of {4, 60} but got 8
Line 507, E:0006: Wrong indentation: expected any of {4, 60} but got 8
Line 508, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 510, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 512, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 513, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 514, E:0006: Wrong indentation: expected any of {4, 60} but got 8
Line 515, E:0006: Wrong indentation: expected any of {4, 60} but got 8
Line 516, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 518, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 519, E:0006: Wrong indentation: expected any of {4, 60} but got 8
Line 520, E:0006: Wrong indentation: expected any of {4, 60} but got 8
Line 521, E:0006: Wrong indentation: expected any of {8, 30, 60, 64} but got 12
Line 522, E:0006: Wrong indentation: expected any of {8, 30, 60, 64} but got 12
Line 523, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 524, E:0006: Wrong indentation: expected any of {4, 60} but got 8
Line 525, E:0006: Wrong indentation: expected any of {4, 60} but got 8
Line 526, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 528, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 530, E:0006: Wrong indentation: expected any of {2, 58} but got 4
Line 531, E:0006: Wrong indentation: expected any of {2, 58} but got 4
----- FILE  :  stream_video_source.js -----
Line 856, E:0006: Wrong indentation: expected any of {14, 41, 45, 66, 70} but got 12

[jonoward]

Yeah this is a weird one, if I clone a fresh copy of master, with no modifications, and run the linter I get a bunch of errors, nearly all of them are the [Missing space before "("] error, e.g. lines like this:
https://github.com/google/shaka-player/blob/master/lib/media/eme_manager.js#L115

Is it possible that the lint rulesets are slightly different on Cygwin? Because for me it seems that nearly all the usages of Promise.catch are failing this test, when I'm not sure if they should (seems it should only apply for actual catch blocks).

I'll try to tweak this PR branch so that it at least fails the linter in the same way that master does. I could run the automated fixjsstyle command, but I'm not sure if you actual intended for these Promise.catch calls to have the extra spaces. I'll try rustle up a linux VM to see if I get different results there. Thanks for the help..

[jonoward]

Right, have pushed some new formatting changes such that the linter is reporting no errors when run on Linux - could you try run the bot again? Thanks

[shaka-bot]

Testing in progress...

[shaka-bot]

All tests passed!