video-dev
Fragment loading/loaded events should accurately reflect "bitrateTest" property
Is your feature request related to a problem? Please describe.
The Fragment object contains a property called bitrateTest that is used to track if a fragment is for a bitrate test. However, this property attempts to do 2 different things:
- track if a fragment is used for a bitrate test
- track the state of the request
I believe this property would be more useful if it just did number 1. It would also allow us to document this property, which may be useful for listeners of the FRAG_LOADING/LOADED event. Today, if a listener of the FRAG_LOADED event tries to query if the fragment loaded was for a bitrate test, they will not be able to, as the property will be false after the request is completed.
Lastly, there is already a member variable in base-stream-controller that tracks the state of the bitrate test. It is not clear why we use that variable in some places in stream-controller and frag.bitrateTest in other places.
Describe the solution you'd like
The solution would be to not set frag.bitrateTest = false after the request is completed. Instead, we can use member variables in any controller that needs to know the state of the request, and continue to use frag.bitrateTest in places where we need to know if the fragment is for a test or not.
Additional context
No response
If working on this, please also consider #7448.
Making a new Fragment instance (or instance of a new sub class like BitrateTestFragment which abstracts away the bitrateTest property) could also be an acceptable solution.
Can you share why handling of FRAG_LOADING/LOADED for bitrate tests is important in your app?
Can you share why handling of
FRAG_LOADING/LOADEDfor bitrate tests is important in your app?
This helps monitor client behavior during media playback start. We want to be able to identify bottlenecks and ensure the bitrate test is actually being performed. We can also look at the results of the test by looking at frag.stats and then assess it's efficacy by looking at how well the BW estimate matched real network conditions.
Fragment property "bitrateTest" should not change and should be documented
Since only one fragment is used to this way, I don't think that having a property on Fragment, of which there can be many instances, is an elegant solution.
I agree that the FRAG_LOADING/LOADED events should signal when they were used only for this purpose. However, we should consider external signaling differently, and the title of this issue should reflect that.
Change the title @robwalch, I think your idea to create a BitrateTestFragment class makes sense, I will look into it.
@robwalch what would you think about introducing the following events: BITRATE_TEST_FRAG_LOADING, BITRATE_TEST_FRAG_LOADED, and triggering them instead of the normal FRAG_LOADING/LOADED? This would actually simplify some things, for example in fragment-tracker.ts onFragLoaded, if frag.bitrateTest is true, we just return. This way we wouldn't even listen to the event in the first place.