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(DASH): Update Configuration for Chromecast devices #7446

Closed
wants to merge 1 commit into from

Conversation

Iragne
Copy link
Contributor

@Iragne Iragne commented Oct 20, 2024

Update Configuration for Chromecast devices to limit the memory consumption on VOD playback and also for live

Fixes #7395

@Iragne Iragne changed the title Update Configuration for Chromecast devices fix(DASH): Update Configuration for Chromecast devices Oct 20, 2024
@shaka-bot
Copy link
Collaborator

Incremental code coverage: 100.00%

@avelad
Copy link
Member

avelad commented Oct 20, 2024

@shaka-bot test

@shaka-bot
Copy link
Collaborator

@avelad: Lab tests started with arguments:

  • pr=7446

@Iragne
Copy link
Contributor Author

Iragne commented Oct 21, 2024

BTW i have 50 as value for CC3 but not sure to properly set it to 50 only for CC3

@avelad avelad requested a review from theodab October 21, 2024 07:06
@avelad avelad added type: bug Something isn't working correctly priority: P2 Smaller impact or easy workaround platform: Cast Issues affecting Cast devices labels Oct 21, 2024
@avelad avelad added this to the v4.12 milestone Oct 21, 2024
@tykus160
Copy link
Member

@Iragne from what I understand, segmentLimit is used only for live playback (check segment_template.js file).

@joeyparrish
Copy link
Member

Yeah, I appreciate this, but I think we have to be careful with a change to the default for all of Chromecast. In particular because we push out new Chromecast SDKs (and their default Shaka versions) in a way that impacts all apps globally, even the ones who have no awareness of what's happening under the hood. It's different when you build your own app and integrate the Shaka Player version of your choice directly.

To be clear, I'm not saying "no" to this, but I think I need to be a little more careful before releasing this change. I think it may need to be coordinated with limits to the availability window, because the initial segment limit only changes what happens at startup, not what accumulates over time. Also, we may want to consider combining these low-memory-related changes into a single flag. Maybe Chromecast Gen 3 isn't the only device that could benefit, so maybe we enable that flag by default for many different device types. And also we might want to avoid that flag by default for newer Cast devices with more memory and higher speeds.

@Iragne
Copy link
Contributor Author

Iragne commented Oct 22, 2024

@Iragne from what I understand, segmentLimit is used only for live playback (check segment_template.js file).

@tykus160 maybe I'm wrong but it's used only under context of } else if (info.segmentDuration) {.
it's true when VOD. I'm happy to double check this again.

@joeyparrish I agree with your approach. And that is true for other devices such as PS, some Tizen, Setupbox, and nesthub...

I'm happy to get all your feedback and propose another approach. I will check how i can combine it with other parameter and devices.

May i can ask how i can properly distinguish the default Chromecast SDK integration compare to the custom one ?

@joeyparrish
Copy link
Member

May i can ask how i can properly distinguish the default Chromecast SDK integration compare to the custom one ?

I'm not sure what you mean by that.

@Iragne
Copy link
Contributor Author

Iragne commented Oct 25, 2024

Sorry for the late answer.

Reading you comment, I understand you would like to make the difference with the shaka player directly avaiable in chromecast compare to a custom cast reciver. Am I wrong ?

if it's the case, we should need to be able to differentiate the 2 use case and i'm not sure to properly know how to do it.

@avelad avelad modified the milestones: v4.12, v4.13 Nov 13, 2024
@joeyparrish
Copy link
Member

@Iragne, sorry for being unclear. I'll try to clarify what I said before:

  1. When you propose a change to a default setting that will affect all Chromecast playbacks, we have to be careful. Very simply, we have to be careful. The default player config for the default player version used in CAF will affect a lot of apps that won't know the details.

  2. The initial segment limit only impacts what happens at startup. It limits the number of segments we parse at first. The availability window, if bigger, will allow more segments to be added to the list later. This means initial segment limit is not the only setting you need to limit segment memory. You also want to carefully limit the availability window.
    availabilityWindowOverride will let you do this, but if used naively, it can also increase the window size.

  3. If we want to limit memory, we should maybe only do it for certain devices or models. Or, offload model detection to the app and have a "low memory" flag that tunes various defaults or places other limits. CAF could set this flag based on the model of the Chromecast receiver device.

@avelad avelad added the status: waiting on response Waiting on a response from the reporter(s) of the issue label Nov 19, 2024
@shaka-bot
Copy link
Collaborator

Closing due to inactivity. If the author would like to continue this PR, they can reopen it (preferred) or start a new one (if needed).

@shaka-bot shaka-bot closed this Nov 26, 2024
@shaka-bot shaka-bot removed the status: waiting on response Waiting on a response from the reporter(s) of the issue label Nov 26, 2024
@Iragne
Copy link
Contributor Author

Iragne commented Nov 28, 2024

Clear sorry i was working on other stuff. Thanks @joeyparrish
I will rework that later

@Iragne Iragne deleted the fix/chromecast-memory branch November 28, 2024 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: Cast Issues affecting Cast devices priority: P2 Smaller impact or easy workaround type: bug Something isn't working correctly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Chromecast (Ultra, Gen3,...) VOD playback stop
5 participants