PulseAudio + Phonon curious issue

[Colin Guthrie]

'Twas brillig, and Harald Fernengel at 20/01/10 13:35 did gyre and gimble:
> Hi,
> 
> On Thursday 14 January 2010 00:15:27 Colin Guthrie wrote:
>> 'Twas brillig, and Colin Guthrie at 13/01/10 10:29 did gyre and gimble:
>>> I'll try and find a fix (tho' I am by no means a gstreamer coder - bit
>>> of a noob there, but I'll try my best seeing as there is not exactly a
>>> queue of people willing to work on the phonon-gstreamer backend right
>>> now :()
>>
>> As with all the most confusing bugs the fix turned out to be trivial.
>> Not sure if it will introduce other potential problems tho', so would
>> appreciate feedback before I commit it upstream.
>>
>> I've created a Mandriva cooker pacakge and as gstreamer is our default
>> backend I'll hopefully know fairly quickly if it's introduced a problem!!
>>
>> Patch can be found here:
>> http://svn.mandriva.com/cgi-bin/viewvc.cgi/packages/cooker/phonon/current/S
>> OURCES/phonon-4.3.80-fix-gstreamer-pulseaudio-deadlock.patch?revision=49107
>> 8&view=markup
>>
>> Please let me know how you get on.
> 
> seems that m_resetNeeded needs to be set to true when stopping media:
> 
> diff --git a/src/3rdparty/phonon/gstreamer/mediaobject.cpp 
> b/src/3rdparty/phonon/gstreamer/mediaobje
> index c2597b5..ceb1de8 100644
> --- a/src/3rdparty/phonon/gstreamer/mediaobject.cpp
> +++ b/src/3rdparty/phonon/gstreamer/mediaobject.cpp
> @@ -622,6 +622,7 @@ void MediaObject::setState(State newstate)
>              changeState(Phonon::StoppedState);
>          } else if (gst_element_set_state(m_pipeline, GST_STATE_READY) != 
> GST_STATE_CHANGE_FAILURE)
>              m_pendingState = Phonon::StoppedState;
> +            m_resetNeeded = true;
>          } else {
>              m_backend->logMessage("phonon state request failed", 
> Backend::Info, this);
>          }
> 
> 
> Otherwise play-stop-play would yield an error message.
> 
> Not sure whether there are other places where we need to set m_resetNeeded to 
> true - Colin, could you review please?

OK, I've taken a look.

I confirm the problem and the logic of the fix makes sense.

I've committed a slightly different fix whereby the m_resetNeeded flag
was set when the message comes back from GST confirming the state change
rather than when we request the change which I think is the "right"
place to put it.

Committed in r1078188

I also moved the invalidateGraph method from the .h into the .cpp as I
kept missing it when searching through the file for m_resetNeeded
occurances and because (IMO) it doesn't belong in the header anyway.

I did that change in r1078189


For ease of patching you can grab the important fix from here:
http://colin.guthr.ie/git/phonon/commit/?id=6fbea9b56a12281819a8c04afd5caa53cfeee39f

Thanks for reporting the issues :)

Col



-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mandriva Linux Contributor [http://www.mandriva.com/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]