minshikshin / google-cast-sdk

Automatically exported from code.google.com/p/google-cast-sdk
0 stars 0 forks source link

Breaking change for setting currentTime in cast_receiver.js 2.0.0 #402

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Using the MPL on a custom receiver, load a PRSS video from sender
2. Override the onMetadataLoaded method and replace event.message.currentTime 
with the time you want to resume at.
3. Call the original onMetadataLoaded (with the event param)

What is the expected output? What do you see instead?
Expected: The video to resume at the point you set in the overridden 
onMetadataLoaded
Actual: The video starts at 0

What version of the product are you using? On what operating system?
Custom Receiver using latest cast_receiver.js. Occurs regardless of sender.

Please provide any additional information below.

In our player we handle the onMetadataLoaded event from the MediaManager which 
passes an event object.

The event object has the following property: event.message.currentTime

We ensure that the property is populated with the currentTime we want to resume 
at. This differs slightly to relying on the playPosition from the sender as we 
have the receiver making some additional decisions on the initial play position 
(so that the logic is common and doesn't have to be in each of the senders).

This used to set the current time. In the old cast_receiver.js you can see this 
as:
a.message.currentTime && this.b && (this.b.currentTime = a.message.currentTime);

In the latest cast_receiver.js it seems this property is ignored completely, so 
our videos always start at 0.

Original issue reported on code.google.com by m...@marcfallows.com on 7 Oct 2014 at 8:04

GoogleCodeExporter commented 9 years ago
Just a quick addition that in the documentation it states 
https://developers.google.com/cast/docs/reference/receiver/cast.receiver.MediaMa
nager#onMetadataLoaded default behaviour is to set the currentTime and call 
sendLoadComplete. It appears that it is now only calling sendLoadComplete.

Original comment by m...@marcfallows.com on 7 Oct 2014 at 8:27

GoogleCodeExporter commented 9 years ago

Original comment by lnicho...@google.com on 7 Oct 2014 at 2:14

GoogleCodeExporter commented 9 years ago
This is what the default behavior states:

Called when load has completed, it can be overridden to handle application 
specific action. The default behavior is to set the currentTime property of the 
media element (if it was provided in the LOAD request), then call 
sendLoadComplete.

The key information is "if it was provided in the LOAD request".

We are still setting the current time to the value passed in during LOAD. Are 
you passing the proper value during LOAD? 

One thing you can do is to set the current time in your media element in your 
onMetadataLoaded override.

Original comment by lnicho...@google.com on 7 Oct 2014 at 6:26

GoogleCodeExporter commented 9 years ago
We have gone the route of setting the currentTime on the media element 
ourselves in onMetadataLoaded. Is that an acceptable solution? (We are getting 
the desired result).

I'm fine with working around this. Ultimately my problem (as described here: 
https://plus.google.com/u/0/+MarcFallows/posts/YXxrn1k1KRb) is that there was a 
breaking change in cast_receiver.js without a version update. To me, this is a 
major issue.

Perhaps that was not your intended use of onMetadataLoaded, but the fact is 
that the public onMetadataLoaded method has changed to ignore 
event.message.currentTime. To me, that constitutes a version bump so we can 
test, verify everything is working, and then opt-in to the new version.

Original comment by m...@marcfallows.com on 8 Oct 2014 at 8:31

GoogleCodeExporter commented 9 years ago
Your solution is acceptable. We will raise your concerns with engineering.

Original comment by lnicho...@google.com on 8 Oct 2014 at 2:10

GoogleCodeExporter commented 9 years ago
Could you provide the code snippet for how you are setting the currentTime?
Also we are curious why you don't set the current time on Load.

Original comment by lnicho...@google.com on 8 Oct 2014 at 5:06

GoogleCodeExporter commented 9 years ago
After we receive the the resume point from the sender's LOAD we kick off a few 
steps to check the validity of the time - and could involve a server request 
(i.e. check if the time is within the credits). At this point we are outside of 
the load handler.

If these checks come back before onMetadataLoaded we were using that method to 
set the currentTime. If the checks come back after (rare), we simply set the 
media element's currentTime directly.

Would you recommend a different approach? Would it be better if hold off on 
calling the original load until I have the correct currentTime? Our concern is 
that this would delay kicking off the load. We are already concerned about the 
time from load to playback using the MPL and PlayReady SmoothStreaming content.

Original comment by m...@marcfallows.com on 9 Oct 2014 at 6:49

GoogleCodeExporter commented 9 years ago
Your case is uncommon but perfectly valid. For this scenario, our 
recommendation is to implement the override in this way:

// Set the current time
<yourMediaElement>.currentTime = <your currenttime>
// Tells the sender that LOAD has been completed
<yourMediaManager>.sendLoadComplete();
// Do not call the default onMetadataLoaded implementation

In this way you do not depend on the default implementation or make assumptions 
about it. We are considering making a change to revert the non backwards 
compatible behavior (so we will still set the currentTime in the default 
implementation) and we want to be sure that if we push it we do not break you.

Original comment by lnicho...@google.com on 9 Oct 2014 at 4:01

GoogleCodeExporter commented 9 years ago
This is the approach I took. We no longer call back to the original 
onMetadataLoaded. Happy to know that this is also your recommended approach in 
our specific case.

Cheers,
Marc

Original comment by m...@marcfallows.com on 13 Oct 2014 at 10:40