Bug 30377 – [GStreamer] Should emit {networkState,readyState,volume,time,size,rate,duration}Changed signals

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Sebastian Dröge (slomo)]

Hi,
the gstreamer media player should emit {networkState,readyState,volume,time,size,rate,duration}Changed signals as appropiate.

networkState and readyState: could probably be implemented based on the GstBus handler: for state changed, buffering and error messages

volumeChanged: Can be done from the notify::volume and notify::mute signals of playbin2 (note: called from the streaming thread usually)

timeChanged: Not sure when this should be called

rateChanged: From some NEWSEGMENT event handler in the video sink if the rate of the segment has changed. And of course after seeks

sizeChanged: From the notify::caps signal of the video sink pad (note: called from the streaming thread usually) if the width, height or PAR has changed.

durationChanged: When a GstMessage of type DURATION is received in the message handler

[Philippe Normand]

Created attachment 42946 [details]
emit volumeChanged and durationChanged upon notification from GStreamer.

[Philippe Normand]

(In reply to comment #0)
> Hi,
> the gstreamer media player should emit
> {networkState,readyState,volume,time,size,rate,duration}Changed signals as
> appropiate.
> 
> networkState and readyState: could probably be implemented based on the GstBus
> handler: for state changed, buffering and error messages
> 

AFAICS these are already emitted during updateStates(), called by the bus message handler.

> volumeChanged: Can be done from the notify::volume and notify::mute signals of
> playbin2 (note: called from the streaming thread usually)
> 

see attached patch ;)

> timeChanged: Not sure when this should be called
> 

It is called after seek and at EOS. Will check if some more is needed.

> rateChanged: From some NEWSEGMENT event handler in the video sink if the rate
> of the segment has changed. And of course after seeks
> 

This is now emitted after succesful rate change (bug 31047). Do you think some more is needed?


> sizeChanged: From the notify::caps signal of the video sink pad (note: called
> from the streaming thread usually) if the width, height or PAR has changed.
> 

Company has a patch for this ;)

> durationChanged: When a GstMessage of type DURATION is received in the message
> handler

See attached patch

[Philippe Normand]

We should indeed emit timeupdate events everytime the playback position changes, not sure about the frequency though. See also http://www.w3.org/TR/html5/video.html#event-timeupdate

[Philippe Normand]

(In reply to comment #3)
> We should indeed emit timeupdate events everytime the playback position
> changes, not sure about the frequency though. See also
> http://www.w3.org/TR/html5/video.html#event-timeupdate

Scratch that, HTMLMediaElement takes care of that.

[Adam Barth]

Attachment 42946 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.h:48:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.h:49:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Done processing WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.h
Done processing WebCore/platform/graphics/gtk/MediaPlayerPrivateGStreamer.cpp
Total errors found: 2

[Philippe Normand]

Created attachment 44060 [details]
emit volumeChanged and durationChanged upon notification from GStreamer.

[WebKit Review Bot]

style-queue ran check-webkit-style on attachment 44060 [details] without any errors.

[Oliver Hunt]

Comment on attachment 44060 [details]
emit volumeChanged and durationChanged upon notification from GStreamer.

r=me

[Gustavo Noronha (kov)]

Landed by Philippe as r52085.