Bug 30377

Summary: [GStreamer] Should emit {networkState,readyState,volume,time,size,rate,duration}Changed signals
Product: WebKit Reporter: Sebastian Dröge (slomo) <slomo>
Component: MediaAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, gustavo, pnormand, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Attachments:
Description Flags
emit volumeChanged and durationChanged upon notification from GStreamer.
none
emit volumeChanged and durationChanged upon notification from GStreamer. oliver: review+

Description Sebastian Dröge (slomo) 2009-10-14 22:40:01 PDT
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
Comment 1 Philippe Normand 2009-11-11 03:17:07 PST
Created attachment 42946 [details]
emit volumeChanged and durationChanged upon notification from GStreamer.
Comment 2 Philippe Normand 2009-11-11 08:06:11 PST
(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
Comment 3 Philippe Normand 2009-11-13 06:50:18 PST
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
Comment 4 Philippe Normand 2009-11-15 23:44:46 PST
(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.
Comment 5 Adam Barth 2009-11-30 12:23:47 PST
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
Comment 6 Philippe Normand 2009-12-01 00:44:25 PST
Created attachment 44060 [details]
emit volumeChanged and durationChanged upon notification from GStreamer.
Comment 7 WebKit Review Bot 2009-12-01 00:48:48 PST
style-queue ran check-webkit-style on attachment 44060 [details] without any errors.
Comment 8 Oliver Hunt 2009-12-11 14:11:01 PST
Comment on attachment 44060 [details]
emit volumeChanged and durationChanged upon notification from GStreamer.

r=me
Comment 9 Gustavo Noronha (kov) 2009-12-14 02:08:28 PST
Landed by Philippe as r52085.