[Webkit-unassigned] [Bug 30000] [GStreamer] Check return values of gst_element_set_state()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30000


Philippe Normand <pnormand at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pnormand at igalia.com




--- Comment #8 from Philippe Normand <pnormand at igalia.com>  2009-12-15 09:01:47 PST ---
(In reply to comment #5)
> (From update of attachment 44519 [details])
> OK.  There is one slight change in logging behavior here, it won't log Play if
> the switch to play fails.  I don't think that matters though.
> 
> Style violation:
>  121             bool changePipelineState(GstState state);
> we don't name arguments when the name doesn't add anything.

Well the variable is used in the code, I will name it newState, which adds
"new" :)

> 
> Shouldn't changePipelineState be private?
> 

It is already a private method. Did I do something wrong?

> And why is it called "pipeline state"?
> 

This is the GStreamer vocabulary, the pipeline is composed of elements, linked
etween themselves. To start playback you set the pipeline state to PLAYING, the
state change propagates through the elements of the pipeline.

> It would appear that this function only works for either PLAY or PAUSE, but I
> don't see you asserting that anywhere.  You might want to ASSERT the inputs.

Will do ;)

Thanks for  the review, I will send a new patch.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.