supernginx / flowplayer-core

Automatically exported from code.google.com/p/flowplayer-core
0 stars 0 forks source link

clip.cuepoints returns an empty array for dynamically added cuepoints #519

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1.
document.getElementsByTagName("button")[0].onclick = function () {
  $f().onCuepoint([5000], function () {
    this.pause();
  });
  console.log($f().getClip().cuepoints);
};

What is the expected output? What do you see instead?
Expected: console shows an array of length 1
Instead: console shows an empty array

Assuming the docs for clip.cuepoints at

http://flowplayer.org/documentation/api/clip.html#properties
"The cuepoint objects of this clip. The property contains all embedded 
cuepoints, cuepoints specified in the configuration and also all cuepoints 
added dynamically using the addCuepoint() method."

actually have the onCuepoint method in mind (addCuepoint does not seem official 
and does nothing, see issue518) the dynamically added cuepoints should be 
retrievable. cuepoints *configured* in a clip's onCuepoint listener can be 
retrieved.

Original issue reported on code.google.com by blacktrashproduct on 18 Apr 2012 at 1:33

GoogleCodeExporter commented 8 years ago
there is an api method called addCuepoints this this what you mean ? this 
functions fine in the  api for the plugins. Perhaps it' not working ?

Original comment by dani...@electroteque.org on 24 Apr 2012 at 12:27

GoogleCodeExporter commented 8 years ago
I already fixed the documentation bug s/addCuepoint()/onCuepoint()/
https://github.com/flowplayer/site/commit/f04ff8d2181dfa3cf534cfe0632c32c6ebe01b
2b

onCuepoint uses api addCuepoints internally, and does work also dynamically, 
i.e.:

$f().onCuepoint([5000], function () {this.pause();});

But after doing that $f().getClip().cuepoints yields [].

i.e. the function works, but seems to empty the readonly variable.

Also it is unclear what the dynamic invocation of onCuepoint does, because as 
far as I know you can only set 1 cuepoint array with 1 handler function. I have 
yet to find out whether dynamic invocation *replaces* an existing cuepoint 
array or *adds to* an existing cuepoint array. And if the latter, what happens 
when you specify a different handler.

Original comment by blacktrashproduct on 24 Apr 2012 at 1:42

GoogleCodeExporter commented 8 years ago
having a look now, apologies, the addCuepoints is internal, it gets fudged into 
the helper event method onCuepoint. 

onCuepoint: [
                [
                    {time: 10000},
                    {time: 20000},
                    {time: 30000}
                ],
                function (clip, cuepoint) {
                    console.log(clip.cuepoints);
                    //alert("cuepoint " + cuepoint + " entered on clip " + clip.url);
                }

            ]

even in this cuepoints is empty. it doesn't seem to be updating the internals 
with the cuepoints though and just on the js level. 

Original comment by dani...@electroteque.org on 25 Apr 2012 at 7:26

GoogleCodeExporter commented 8 years ago
It's being emptied internally on the onBegin event. 

Original comment by dani...@electroteque.org on 25 Apr 2012 at 8:18

GoogleCodeExporter commented 8 years ago
Bug somewhere, if you configure within a playlist config not the clip config it 
is fine

ie

playlist: [
            {
                provider: 'rtmp',
                url: ''
                onCuepoint: [
                    [
                        {time: 10000},
                        {time: 20000},
                        {time: 30000}
                    ],
                    function (clip, cuepoint) {
                        console.log(clip.cuepoints);

                    }

                ]
            }
]

Original comment by dani...@electroteque.org on 25 Apr 2012 at 8:33

GoogleCodeExporter commented 8 years ago
Couple of possible fixes, 

1) the javascript is obtaining the common clip and configuring on that, not the 
actual clip because its index is -1 not 0. 
2) min index check in flowplayer itself. 

Original comment by dani...@electroteque.org on 25 Apr 2012 at 8:53

GoogleCodeExporter commented 8 years ago
I don't think we should rush this. There are other issues which are much more 
important. Let me investigate and think a little bit more, because the API is 
not clear here anyway - like how the handler function is, errmmh, handled etc.

Original comment by blacktrashproduct on 25 Apr 2012 at 9:15

GoogleCodeExporter commented 8 years ago
Its not even adding to the common clip, the design is clip { } adds some common 
things to all playlist items. however this.getCommonClip does not return 
cuepoints at all. I feel cuepoints is playlist item specific, so restricting 
the index to 0 for the first clip item, will add the cuepoints properly to the 
specific clip. This index can be fixed up to a minium of 0 not -1 in the js or 
the internals. 

Original comment by dani...@electroteque.org on 25 Apr 2012 at 9:18

GoogleCodeExporter commented 8 years ago
The onCuepoint is simply a helper with a callback method, its calling the 
flowplayer api method addCuepoints internally. 

Original comment by dani...@electroteque.org on 25 Apr 2012 at 9:19

GoogleCodeExporter commented 8 years ago
Let me know how this fix fares for you cheers

http://dl.dropbox.com/u/3394987/flowplayer.commercial-3.2.10.zip

Original comment by dani...@electroteque.org on 25 Apr 2012 at 9:56

GoogleCodeExporter commented 8 years ago
Hi guys, Thanks for working on this!  Cuepoints are the most important feature 
set of flowplayer to us.  Manipulating them with javascript is of utmost 
importance.

Can you please tell me what the fix is in 
http://dl.dropbox.com/u/3394987/flowplayer.commercial-3.2.10.zip ?

Original comment by f...@b3dmultitech.com on 26 Apr 2012 at 5:25

GoogleCodeExporter commented 8 years ago
Again, please do not rush this issue!

Currently the _only_ bug is that the readonly clip.cuepoints variable becomes 
empty when you call onCuepoint *dynamically*.

The onCuepoint function works fine both as configured event and dynamically.

But as onCuepoint is quite involved and extremely customizable we have to 
investigate and document certain corner cases first.

Example:

An array of cuepoints is *configured* in the player configuration.
What happens when you then call onCuepoint *dynamically* with a _different_ 
event handler? Are the dynamically added cuepoints added to the configured 
ones? Does the dynmaically added handler take over (there is only one array of 
cuepoints per clip, ergo only one handler)?

So, again, let's not rush this. In almost any case one should be able to work 
around *this* bug by rearranging one's scripts.

Original comment by blacktrashproduct on 26 Apr 2012 at 8:41

GoogleCodeExporter commented 8 years ago
If you want cuepoints arrays returned, it needs to be setup within the playlist 
config as explained above, within the clip config it's global for all playlist 
items. When configured inside the clip it is configuring, but its not actually 
been added to the playlist clip item, therefore it has no cuepoints configured. 

Confirmed within onLoad 

onLoad: function() {
            this.onCuepoint([1100, 2400, 3350], function(clip, cuepoint) {
                // put some logging information into firebug console
                //console.log("cuepoint entered", clip, cuepoint);
                 console.log(clip);
            });
        },

It is empty also because all of these options are on the common global clip 
config. This should be accessible via this.getCommonClip().cuepoints but isn't. 

Original comment by dani...@electroteque.org on 26 Apr 2012 at 6:56

GoogleCodeExporter commented 8 years ago
OK there is now two ways to do this, 

expose the api method to get the common clip correctly and not the javascript 
built one as the cuepoints is not added to this, when I run getCommonClip again 
the cuepoints are there with the generated information

cuepoints: Array[3]
0: Object
lastFireTime: -1
parameters: Object
time: 1100
__proto__: Object
1: Object
lastFireTime: -1
parameters: Object
time: 2400
__proto__: Object
2: Object
lastFireTime: -1
parameters: Object
time: 3400
__proto__: Object
length: 3

, the only other way to get the generated information back is returning it 
after a call to addCuePoints, and it could be added to the javascript 
commonClip object. The easiest solution is an api method callback. The reason 
why the cuepoints are added to the clips in a playlist is because of the 
getplaylist callback. 

As per usual for clip specific cuepoints , ie dependant on durations can be 
done in the playlist config instead. 

Original comment by dani...@electroteque.org on 26 Apr 2012 at 7:39

GoogleCodeExporter commented 8 years ago
If you have a moment let me know what you think the best way about this is, 
regardless it would need changes to the core and the js api anyway. 

Original comment by dani...@electroteque.org on 28 Apr 2012 at 4:03

GoogleCodeExporter commented 8 years ago
What did you want to do about this. There is obviouslly no way functionally to 
get the cuepoints. I reckon documentation change to add the onCuePoints helper 
on the clips in the playlist. 

Original comment by electrot...@gmail.com on 12 May 2012 at 4:05

GoogleCodeExporter commented 8 years ago
Apart from clip.cuepoints it seems unclear what
$f().onCuepoint([5000], function () {this.pause();});
does exactly.
It works with one clip, but what happens when you have a playlist running?
Does it add cuepoints to all items (i.e. commonclip)?
What happens with already configured cuepoints?

The biggest source of confusion is that the api allows CLIP events to be set 
directly with $f().

$f().onCuepoint() vs. $f().getClip(0).onCuepoint() - I guess $f().onCuepoint() 
targets the common clip object. But this is already complicated for 
non-customizable events: $f().onStart() adds to $f().getClip(0).onStart():
http://flowplayer.org/documentation/events/index.html#multiple-event-listeners

Original comment by blacktrashproduct on 13 May 2012 at 10:04

GoogleCodeExporter commented 8 years ago
Hi I just ran some tests 

add this to both a playlist item and common clip

onCuepoint: [
              // each integer represents milliseconds in the timeline
               [5000],

               // this function is triggered when a cuepoint is reached
               function(clip, cuepoint) {
                   alert("cuepoint " + cuepoint + " entered on clip " + clip.url);
               }
           ],  

the playlist onCuepoint gets fired off first then the common clip one gets 
fired off, so they don't override eachother but get called for that start time. 

Original comment by electrot...@gmail.com on 18 May 2012 at 5:10

GoogleCodeExporter commented 8 years ago
Right, a bit à la http://flowplayer.org/demos/events/bubbling.html

I have now changed the docs for the clip.cuepoints property, and also moved it 
only into the api/clips section as a READ-ONLY property - you cannot 
_configure_ clip.cuepoints, so it was documented even in the wrong place.
https://github.com/flowplayer/site/commit/a3166c927071be3f32a4fafb951a299bfde7cc
be

I'm almost tempted to omit it altogether from the public properties, like 
backBufferLength, durationFromMetadata and friends, but I think now the docs 
reflect the actual state - more or less.

Original comment by blacktrashproduct on 1 Jun 2012 at 12:49

GoogleCodeExporter commented 8 years ago
You might need to extend the wording that this is available for cuepoint events 
added into the specific playlist items not on the common clip object.

Original comment by dani...@electroteque.org on 1 Jun 2012 at 7:31

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Yes, I intend to do that. I just want to check whether this is true for ALL 
read-only clip properties: fullDuration cannot be common, clip.metaData, 
clip.extension etc. are only valid for specific clips by definition.

I haven't checked - or don't remember:

If you script:

$f().getClip(0).onCuepoint(100, function () { something });

will that be listed afterwards?

Original comment by blacktrashproduct on 1 Jun 2012 at 12:37