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

Fix the waiting process was called too much. #2548

Closed
wants to merge 2 commits into from
Closed

Fix the waiting process was called too much. #2548

wants to merge 2 commits into from

Conversation

mofneko
Copy link
Contributor

@mofneko mofneko commented Mar 10, 2017

The onLoadCompletedhls calling processLoadedPlaylist is triggered by loadPlaylist, but it fires at multiple events.
As a result, waiting for an excessive playlist rereading process not compliant with HLS spec v 20, section 6.3.4 "for more information on media playlist refreshing" was called. Therefore, since the acquisition process was excessive on the server side, measures were taken.

As for the correction contents, we implemented block processing with a flag between process wait start and wait end of processLoadedPlaylist to comply with hls reload specification.

Before:

22:43:52.298 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 31471e2
22:43:52.299 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:43:53.524 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 31471e2
22:43:53.524 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:43:54.013 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 31471e2
22:43:54.013 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:43:54.603 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 3bd02fc
22:43:54.603 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:43:55.416 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 31471e2
22:43:55.973 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 31471e2
22:43:55.973 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:43:56.353 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 3bd02fc
22:43:56.354 processLoadedPlaylist(postDelayed) refreshDelayUs: 6000000
22:43:56.440 processLoadedPlaylist call isPending: true MediaPlaylistBundle: 31471e2
22:43:56.440 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:43:56.607 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 31471e2
22:43:56.607 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:43:57.089 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 31471e2
22:43:57.089 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:43:57.682 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 3bd02fc

After:

22:34:40.251 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 7e327ef
22:34:40.251 processLoadedPlaylist(postDelayed) refreshDelayUs: 6000000
22:34:40.387 processLoadedPlaylist call isPending: true MediaPlaylistBundle: 68bca2e
22:34:41.434 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 68bca2e
22:34:41.978 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 68bca2e
22:34:41.978 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:34:45.066 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 68bca2e
22:34:45.557 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 68bca2e
22:34:45.557 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:34:46.338 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 7e327ef
22:34:46.338 processLoadedPlaylist(postDelayed) refreshDelayUs: 6000000
22:34:46.423 processLoadedPlaylist call isPending: true MediaPlaylistBundle: 68bca2e
22:34:48.655 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 68bca2e
22:34:48.655 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:34:51.748 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 68bca2e
22:34:52.276 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 68bca2e
22:34:52.276 processLoadedPlaylist(postDelayed) refreshDelayUs: 3000000
22:34:52.423 processLoadedPlaylist call isPending: false MediaPlaylistBundle: 7e327ef
22:34:52.423 processLoadedPlaylist(postDelayed) refreshDelayUs: 6000000
22:34:52.490 processLoadedPlaylist call isPending: true MediaPlaylistBundle: 68bca2e
  • IsPending is the flag implemented this time when processLoadedPlaylist is executed
  • RefreshDelayUs is the configured wait time at runtime
  • MediaPlaylistBundle: xxxxxxx is the instance id of the running MediaPlaylistBundle
  • The # EXT-X-TARGETDURATION of our playlist was set to 6.

Please note that postDelayed is running if isPending: true.

@mofneko
Copy link
Contributor Author

mofneko commented Mar 13, 2017

I'm sorry, the explanation was missing.
The above log is output when processLoadedPlaylist is called the processLoadedPlaylist method.
processLoadedPlaylist (postDelayed) is not repelled by isPendingLoadPlaylist judgment,
It is output when playlistRefreshHandler.postDelayed is processed.

@AquilesCanta
Copy link
Contributor

Thanks for the pull request! This is something I had noticed some time ago but hadn't gotten to measure. We already have an internal fix for this which prevents the initiating the premature load, instead of discarding the already loaded playlists. Please let me know if you still notice too many requests. There is one minor optimization that can still be done, but I don't know whether it is worth it, though.

@mofneko
Copy link
Contributor Author

mofneko commented Mar 14, 2017

I appreciate your reply!
The one we surveyed the other day is the release-v2 branch, but is the dev-v2 branch that has been corrected?
It would be great if you could tell me about the commit of the target if possible. And let me investigate it again.

@AquilesCanta
Copy link
Contributor

No, we have not yet pushed the fix to GitHub, it will happen soon. A reference to the commit should be automatically posted here.

@mofneko
Copy link
Contributor Author

mofneko commented Mar 14, 2017

OK. I will wait for it. And the results are reported here.

ojw28 pushed a commit that referenced this pull request Mar 15, 2017
This greatly reduces the amount of server requests issued by
the playlist tracker.

Issue:#2548

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=150055046
@ojw28
Copy link
Contributor

ojw28 commented Mar 15, 2017

Fix is ref'd above. Closing this.

@ojw28 ojw28 closed this Mar 15, 2017
@mofneko
Copy link
Contributor Author

mofneko commented Mar 16, 2017

Thank you.
I investigated it and confirmed that the result was the expected value.
By the way, I have a doubt about the change this time, so I made more PR.
See, #2564

@mofneko mofneko deleted the origin/dev-v2-fix-hls-playlist-loading-too-much branch March 16, 2017 07:10
@google google locked and limited conversation to collaborators Jul 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants