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

MediaController’s play_media should not set current_time to 0 by default #439

Closed
andrewshadura opened this issue Dec 27, 2020 · 16 comments · Fixed by #446
Closed

MediaController’s play_media should not set current_time to 0 by default #439

andrewshadura opened this issue Dec 27, 2020 · 16 comments · Fixed by #446

Comments

@andrewshadura
Copy link

andrewshadura commented Dec 27, 2020

The documentation for the Load command says currentTime is optional:

Seconds since beginning of content. If the content is live content, and position is not specifed, the stream will start at the live position

This is vital to the ability to cast live streams and is not currently possible without deriving a separate controller overriding play_media. Please consider defaulting to None e.g. in this manner:

diff --git a/pychromecast/controllers/media.py b/pychromecast/controllers/media.py
index cc47db4..54f9904 100644
--- a/pychromecast/controllers/media.py
+++ b/pychromecast/controllers/media.py
@@ -492,7 +492,7 @@ class MediaController(BaseController):
         content_type,
         title=None,
         thumb=None,
-        current_time=0,
+        current_time=None,
         autoplay=True,
         stream_type=STREAM_TYPE_BUFFERED,
         metadata=None,
@@ -560,7 +560,7 @@ class MediaController(BaseController):
         content_type,
         title=None,
         thumb=None,
-        current_time=0,
+        current_time=None,
         autoplay=True,
         stream_type=STREAM_TYPE_BUFFERED,
         metadata=None,
@@ -628,7 +628,8 @@ class MediaController(BaseController):
                 "media": media,
                 MESSAGE_TYPE: TYPE_LOAD,
             }
-        msg["currentTime"] = current_time
+        if current_time is not None:
+            msg["currentTime"] = current_time
         msg["autoplay"] = autoplay
         msg["customData"] = {}
@emontnemery
Copy link
Collaborator

This seems reasonable enough, can you please explain what difference it makes though?
Do you want to deduce if it's live media by testing if current_time is None?

@andrewshadura
Copy link
Author

No, I want to be able to play the live stream at the live stream position, not at two or three hours in the past (which is what 0 corresponds to).

@andrewshadura
Copy link
Author

When I’m playing live TV streams, None plays what’s currently being aired, 0 plays the recording of however much the TV stream allows to seek into the past, in my case it’s about three hours.

@emontnemery
Copy link
Collaborator

OK. To make this a non-breaking change, I think the following change is enough, i.e. no change of default values:

                 "media": media,
                 MESSAGE_TYPE: TYPE_LOAD,
             }
-        msg["currentTime"] = current_time
+        if current_time is not None:
+            msg["currentTime"] = current_time
         msg["autoplay"] = autoplay
         msg["customData"] = {}

@andrewshadura
Copy link
Author

andrewshadura commented Jan 8, 2021 via email

@emontnemery
Copy link
Collaborator

For a non live stream, does not setting currentTime have the same result as setting it to 0?

@andrewshadura
Copy link
Author

I believe so; non-live contents starts playing from the beginning.

@emontnemery
Copy link
Collaborator

Can you give some examples of live streams where you see the problem (for testing)?

@emontnemery
Copy link
Collaborator

To be clear: Since what you propose is a breaking change, I'd really like to have some examples of live streams where setting currentTime to 0 starts playback from the past as you explain. Would that be possible?

@andrewshadura
Copy link
Author

I’m afraid I’m not able to share the link since it’s not a public stream, and I’ve not tried watching other TV streams to be honest.

I don’t understand why you consider this a breaking change: watching live TV streams with a "backlog" didn’t work before but will work with this change, nothing else is affected by it.

@andrewshadura
Copy link
Author

Okay, found you a public stream where this behaviour can be observed: http://mfe.cliptv.az/dash/BBC_World_News_SD.ism/playlist.mpd

Here, the live stream is playing on the laptop, and the current_time=0 is playing on the Chromecast:

2021-01-11 17 01 14

@andrewshadura
Copy link
Author

It’s not two or three hours difference, as with my other non-public stream, but a difference nevertheless.

@emontnemery
Copy link
Collaborator

Thanks!

Can you please explain again why what I proposed here is not good enough: #439 (comment)
What's the use case where it's not acceptable to pass None to currentTime (aside from the default behavior being more reasonable with the change)?

@andrewshadura
Copy link
Author

It is acceptable, but I don’t understand why keep incorrect default behaviour (which goes against the spec) which I doubt anything depends on? What worked before, will still work since non-live videos default to 0 anyway.

@andrewshadura
Copy link
Author

If you want even better compatibility, feel free to make it depend on the resource type being a live stream.

@emontnemery
Copy link
Collaborator

Thanks for the patience, implemented in #446

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 a pull request may close this issue.

2 participants