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

Raise transcoding kill timeout. #11643

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kevincox
Copy link

@kevincox kevincox commented May 14, 2024

Changes
The Chromecast client only pings every 10 seconds, so having a 10 second timeout will result in spurious killing any time there is slight deviation in the polling. Set to a much safer value so that transcoding will only get killed if there is a serious hang somewhere.

Issues
Fixes: #11640

The Chromecast client only pings every 10 seconds, so having a 10 second timeout will result in spurious killing any time there is slight deviation in the polling. Set to a much safer value so that transcoding will only get killed if there is a serious hang somewhere.
@kevincox
Copy link
Author

kevincox commented May 14, 2024

This timeout appears to have been introduced by @crobibero in #3460.

@gnattu
Copy link
Member

gnattu commented May 14, 2024

I believe this is not the problem if your playback does not even start

@kevincox
Copy link
Author

It isn't the main problem, but it is a problem. The fact is that 10s pings with a 10s timeout isn't going to work reliably.

@gnattu
Copy link
Member

gnattu commented May 14, 2024

It isn't the main problem, but it is a problem. The fact is that 10s pings with a 10s timeout isn't going to work reliably.

Except we are not:

https://github.com/jellyfin/jellyfin-chromecast/blob/b8f75fcf5c1a21633932273ed64eef67b32faac4/src/components/jellyfinActions.ts#L158

On chromecast we are pinging at 5s.

@kevincox
Copy link
Author

That isn't what I am seeing in the nginx access logs: https://github.com/jellyfin/jellyfin/files/15312949/jellyfin.txt

@kevincox
Copy link
Author

Trying the Unstable channel the pings seem to be usually a bit over 5s, but just copying the last few I did notice a missed one at 11s. Even then a bit more buffer would be good.

  • 23:54:07
  • 23:54:13 (6s)
  • 23:54:18 (5s)
  • 23:54:23 (5s)
  • 23:54:28 (5s)
  • 23:54:39 (11s)
  • 23:54:44 (5s)
  • 23:54:49 (5s)

@kevincox
Copy link
Author

kevincox commented May 15, 2024

Ok, in that trace Sessions/Playing/Progress was hit in the gap which appears to be wired up to the ping endpoint. 5s with 10s timeout still seems too tight for me as one missed request can result in a timeout (What if the request hangs?), but it seems that in good conditions the Unstable Chromecast client shouldn't have issues with this.

I double checked though and the logs from the Stable Chromecast don't have a progress update in the gap, so they are still only going every 10s.

@jellyfin-bot
Copy link
Contributor

This pull request has merge conflicts. Please resolve the conflicts so the PR can be successfully reviewed and merged.

@jellyfin-bot jellyfin-bot added the merge conflict Merge conflicts should be resolved before a merge label May 15, 2024
@cvium
Copy link
Member

cvium commented May 16, 2024

Does Chromecast even use Progressive streams? I don't think so, but not sure.

@gnattu
Copy link
Member

gnattu commented May 16, 2024

Does Chromecast even use Progressive streams? I don't think so, but not sure.

From the Chromecast Client source code it does, but that pings the server at 5 second interval which should be good enough.

@jellyfin-bot jellyfin-bot removed the merge conflict Merge conflicts should be resolved before a merge label May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Issue]: Transcoding job killed before playback starts.
4 participants