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

duplicated audio requests #1009

Closed
kuznetcoff777 opened this issue Sep 6, 2017 · 11 comments
Closed

duplicated audio requests #1009

kuznetcoff777 opened this issue Sep 6, 2017 · 11 comments
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@kuznetcoff777
Copy link

Have you read the FAQ and checked for duplicate issues: yes

What version of Shaka Player are you using: v2.2.1-debug

Can you reproduce the issue with our latest release version: yes

Can you reproduce the issue with the latest code from master: did not check

Are you using the demo app or your own custom app: demo

If custom app, can you reproduce the issue using our demo app:

What browser and OS are you using:chrome/win7x64

What are the manifest and license server URIs:
(you can send the URIs to shaka-player-issues@google.com instead, but please use GitHub and the template for the rest)

What did you do? Opened https://shaka-player-demo.appspot.com/demo/#asset=//download.tsi.telecom-paristech.fr/gpac/DASH_CONFORMANCE/
doubled-audio-requests-1
TelecomParisTech/mp4-live-periods/mp4-live-periods-mpd.mpd;lang=ru
After that in console i inserted
shakaDemo.localPlayer_.configure({abr: {restrictions: {maxBandwidth: 200000}}})
Player downloaded few audio/video segments
After that in console i inserted
shakaDemo.localPlayer_.configure({abr: {restrictions: {maxBandwidth: 1000000}}})

What did you expect to happen? Player fetch new video segments, but audio segments will not refetch.

What actually happened? At first player downloaded low profile video segments with audio segments (25-26). After that player fetched higher profile video segments and refeteched the same 25-26 audio segments. So it is not not effectively.

@vaage
Copy link
Contributor

vaage commented Sep 6, 2017

@kuznetcoff777 I see the problem you are talking about, but I don't understand how this would happen under normal use. In what scenarios would you be re-configuring the player while it is playing back content?

@vaage vaage added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Sep 6, 2017
@kuznetcoff777
Copy link
Author

For example while changing resolution of player. If small player, then download only SD profiles, if huge player, then download all up to HD profiles

@vaage vaage added type: bug Something isn't working correctly and removed status: waiting on response Waiting on a response from the reporter(s) of the issue needs triage labels Sep 7, 2017
@vaage vaage self-assigned this Sep 8, 2017
@vaage vaage added this to the v2.3.0 milestone Sep 11, 2017
@kuznetcoff777
Copy link
Author

One more important thing i want to notice, if maxBandwidth changed (for example player changed to hires), then player download hd segemtns (while palyer still have SD buffer) so player stalls for a couple of seconds. It irritates. It is good from the point of quality of picture, but bad from point of player stalls. I think it will be good to have some parameter, for if it is true then player downloads new segments, if false, then player should play its previous buffer and only after that goes to another resolution.

@vaage
Copy link
Contributor

vaage commented Sep 15, 2017

@kuznetcoff777 , I would suggest opening a separate issue for delaying the change from SD to HD as that increases the scope of this issue and enters into the space of an enhancement.

@TheModMaker
Copy link
Contributor

So here's the problem: when you call configure(), we switch to the different tracks and we clear the buffer. This causes the video to pause while we buffer the new content. This is important for changing languages, since you want the changes to be visible/audible immediately.

The restrictions object is meant to restrict what tracks can be played. There are two kinds of restrictions, one that is applied globally and one for ABR. The global one is meant for giving Shaka Player more info about the platform or DRM. For example, if the device can't play UHD, you can tell us not to try. So if you change the restrictions, it is important to do so immediately since the content we are playing may not play well (or at all for protected content).

When changing the restrictions for ABR, it may not make much sense to clear the buffer since the restrictions don't apply as strictly. So I think we should make an exception and not clear the buffer when changing the ABR restrictions; but keep the old behavior when changing the global restrictions or changing languages/roles.

@kuznetcoff777
Copy link
Author

@TheModMaker So do i need to open new issue?

@TheModMaker
Copy link
Contributor

No, it is all part of the same issue.

shaka-bot pushed a commit that referenced this issue Sep 27, 2017
It appears that we were clearing the buffers when we re-configured the
ABR manager. This was not necessary, so instead we do not clear the
buffers so that any buffered data can be used.

Change-Id: I3b57278fc47eed4d36e91e0e59211f374dfdfc06
Issue: #1009
@vaage
Copy link
Contributor

vaage commented Sep 27, 2017

@kuznetcoff777 I have submitted a fix for the bug into master. However I am going to keep this issue open until I add tests to ensure we do not regress in the future.

@kuznetcoff777
Copy link
Author

Thanks! It works!

@joeyparrish
Copy link
Member

I'm cherry-picking the fix to v2.2.3. This issue is still open until we land appropriate regression tests to go along with the fix.

joeyparrish pushed a commit that referenced this issue Oct 16, 2017
It appears that we were clearing the buffers when we re-configured the
ABR manager. This was not necessary, so instead we do not clear the
buffers so that any buffered data can be used.

Change-Id: I3b57278fc47eed4d36e91e0e59211f374dfdfc06
Issue: #1009
joeyparrish pushed a commit that referenced this issue Nov 1, 2017
When changing restrictions, we don't need to clear the buffers. This tests
checks that when we change the config to change the ABR restrictions,
we do not clear the buffers.

Change-Id: I04e360c7fdfa008d7ae9a8050974bcd28f1d3388
Closes: #1009
@joeyparrish
Copy link
Member

Regression test cherry-picked for v2.2.5.

@shaka-project shaka-project locked and limited conversation to collaborators Mar 22, 2018
@shaka-bot shaka-bot added the status: archived Archived and locked; will not be updated label Apr 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Projects
None yet
Development

No branches or pull requests

5 participants