Bug 39797 - module-cork-music-on-phone unable to send CORK event to pulsesink for PA_STREAM_START_CORKED streams
Summary: module-cork-music-on-phone unable to send CORK event to pulsesink for PA_STRE...
Status: RESOLVED MOVED
Alias: None
Product: PulseAudio
Classification: Unclassified
Component: modules (show other bugs)
Version: unspecified
Hardware: All All
: medium major
Assignee: pulseaudio-bugs
QA Contact: pulseaudio-bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-03 04:23 UTC by Himanshu Chug
Modified: 2018-07-30 10:19 UTC (History)
4 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Himanshu Chug 2011-08-03 04:23:58 UTC
Steps to reproduce:

1. load module-cork-music-on-phone

2. play music from gst-launch with media role="phone"
e.g gst-launch playbin2 uri=file:///userdata/media/phone/Disco.mp3 audio-sink="pulsesink role="music""

3. play music from gst-launch with media role="music"
gst-launch playbin2 uri=file:///userdata/media/phone/Tere.mp3 audio-sink="pulsesink role="phone""


Expected Results:

"music" stream should mute and get corked , with gst-pipeline received event to PAUSE the "music" stream pipeline.

Actual Results:

"music" stream is getting muted but the "music" stream is also getting mixed with "phone" stream , and gst pipeline for "music" stream is NOT getting paused.

Since the "music" stream is getting muted , so we cannot hear the mixed stream output, but one can easily figure out the problem by enabling gst logs and with gst pipeline state which remains in PLAYING state.


Some extra Information:

I have tried putting logs in gst pulsesink gst_pulsering_stream_event_cb() function, but it is not getting CORK request from pulseaudio in this particular senario, 

Reverse Usecase i.e playing music stream first and then "phone" stream next is working fine and I can see CORK request callback in gst_pulsering_stream_event_cb() 

So I suspect the issue is with PA_STREAM_START_CORKED flagged streams, which start in corked state ("music" stream in our case) and immediately were asked again to go to CORKED state with NO effect ?
Comment 1 Colin Guthrie 2011-08-03 04:27:38 UTC
The last time I looked the GST pulsesink code simply didn't listen for the cork event we send. If that is still the case, this is just a GST bug (or lack of feature) rather than a PA bug.

Arun will know.... :p
Comment 2 Himanshu Chug 2011-08-03 04:33:59 UTC
(In reply to comment #1)
> The last time I looked the GST pulsesink code simply didn't listen for the cork
> event we send. If that is still the case, this is just a GST bug (or lack of
> feature) rather than a PA bug.
> 
> Arun will know.... :p


Hi Colin, 

Reverse Usecase i.e playing music stream first and then "phone" stream next is
working fine and I can see CORK request callback in gst_pulsering_stream_event_cb(), 

So the callback for receiving events is available in gst-pulsesink , but the event is somehow not reached to GST in this case.

Best Regards,
Himanshu
Comment 3 Himanshu Chug 2011-08-03 05:19:26 UTC
(In reply to comment #2)
> (In reply to comment #1)
> > The last time I looked the GST pulsesink code simply didn't listen for the cork
> > event we send. If that is still the case, this is just a GST bug (or lack of
> > feature) rather than a PA bug.
> > 
> > Arun will know.... :p
> 
> 
> Hi Colin, 
> 
> Reverse Usecase i.e playing music stream first and then "phone" stream next is
> working fine and I can see CORK request callback in
> gst_pulsering_stream_event_cb(), 
> 
> So the callback for receiving events is available in gst-pulsesink , but the
> event is somehow not reached to GST in this case.
> 
> Best Regards,
> Himanshu


we need to install the new property "role" to get this support in gst-pulsesink
like here:

static void
gst_pulsesink_class_init (GstPulseSinkClass * klass)
{
...........

  g_object_class_install_property (gobject_class, PROP_MEDIA_ROLE,
      g_param_spec_string ("role", "Role",
          "The PulseAudio stream media role", DEFAULT_MEDIA_ROLE,
          G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS))

}

otherwise to reproduce this issue, we can modify paplay app to create a new stream with role property using 

pa_proplist_sets(pl, PA_PROP_MEDIA_ROLE, "phone");
pa_stream_new_with_proplist (,,pl);

and creating other stream ( for music role)from gst-pulsesink, I guess the default role of a gst stream is music, so that way it work out.
Comment 4 Colin Guthrie 2011-08-03 05:25:29 UTC
You can also do:

PULSE_PROP="media.role=phone" gst-launch......


See http://pulseaudio.org/wiki/FAQ#WhatenvironmentvariablesdoesPulseAudiocareabout


As for the underlying problem, I've seen this before too.

There is also a more fundamental issue surrounding it as there is a stream flag (PA_STREAM_START_CORKED) which means the client typically triggers an uncork at startup.

Chances are when the stream is first seen, it's already corked (due to the flag) and then it uncorks itself as per normal....

Obviously we need to avoid this scenario, but I'm not seeing the obvious fix for it right now! Thoughts welcome!
Comment 5 Arun Raghavan 2011-08-19 02:42:23 UTC
I've always looked at this as acceptable behaviour. If you're on a call, the expected reason that a new sound is played are:

1. It's an event sound -- short-lived, and you probably want to hear it

2. It's a stream started by something the user did -- the user initiated it and thus we really shouldn't be forcing it to be paused (he/she's bored in a conf call and wants some music in the background, for example)

Of course, this only caters to the desktop case. For more specialised devices, different rules (depending on the use case) will need to be applied.
Comment 6 Colin Guthrie 2011-08-19 03:11:26 UTC
I do somewhat agree that it's acceptable behaviour but I also think that our infrastructure should allow all use-cases and that it should be up to policy to decide the behaviour, not be dictated by certain limitations.

I'd therefore like to find some way to handle the corking stuff slightly differently, e.g. by introducing another state (PA_SINK_INPUT_INITIAL_CORK or something) or by introducing a mechanism to track things better.

Sadly I did discuss an approach for solving this with Lennart the other day but sunny days wondering the streets of Berlin were not conducive to good note taking so I've kinda forgotten what we came up with :s


It'll come back soon and I'll document it here for further discussion.
Comment 7 Tanu Kaskinen 2011-08-19 06:52:25 UTC
I'm not sure if the following solution works in this case. If I understood correctly, the problem here is that module-cork-music-on-phone sends a cork request to the client, but the client uncorks the stream soon after that. To solve this at the server end we would have to know why the client is uncorking the stream - is it reacting to a request from module-cork-music-on-phone or something else. This information is not available, and I don't see any solution to this other than extending the client API in some way.

Anyway, here's a proposal for handling corking, suspending and muting (I'll use corking as the example, but the same principle applies to suspending and muting too):

Corking should work so that when some component wants to cork a stream, it should create a "cork object". The cork object gets attached to the stream. The stream may have multiple cork objects attached; there may be multiple components wanting to cork a stream at the same time. The stream should stay corked as long as there is at least one cork object attached to it.
Comment 8 Himanshu Chug 2011-08-23 01:22:19 UTC
(In reply to comment #7)
> I'm not sure if the following solution works in this case. If I understood
> correctly, the problem here is that module-cork-music-on-phone sends a cork
> request to the client, but the client uncorks the stream soon after that. 

The CORK request is not reaching to the client (gst pulse-sink in this case). 

Please check the GST Logs for "music" stream in two different scenarios below:

(I have kept prints for ENTRY/EXIT/CORK/UNCORK in gst_pulsering_stream_event_cb() function )


I. 
Playback first music stream and then phone stream arrives: Works fine and we are getting CORK request for music stream.

# gst-launch playbin2 uri=file:///userdata/media/phone/Music/Raindrops.mp3 audio
-sink="pulsesink role="music""
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
DBG: pa_stream_new_with_proplist: 99: name = Playback Stream
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstPulseSinkClock

 gst_pulsering_stream_event_cb: ENTRY

 gst_pulsering_stream_event_cb: got request for CORK
Setting state to PAUSED as requested by /GstPlayBin2:playbin20/GstPlaySink:playsink0/GstBin:abin/GstBin:bin0/GstPulseSink:pulsesink0...

 gst_pulsering_stream_event_cb: EXIT
^CCaught interrupt -- handling interrupt.
Interrupt: Stopping pipeline ...
Execution ended after 22206207274 ns.
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...

II. 

Playback Order: Phone stream already ongoing and then Music stream starts: the streams were getting mixed, We are NOT getting CORK request here.
  Getting "got request for UNCORK" after killing "phone" stream.

# gst-launch playbin2 uri=file:///userdata/media/phone/Music/Raindrops.mp3 audio
-sink="pulsesink role="music""
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
DBG: pa_stream_new_with_proplist: 99: name = Playback Stream
Pipeline is PREROLLED ...
Setting pipeline to PLAYING ...
New clock: GstPulseSinkClock

 gst_pulsering_stream_event_cb: ENTRY

 gst_pulsering_stream_event_cb: got request for UNCORK

 gst_pulsering_stream_event_cb: EXIT
Setting state to PLAYING as requested by /GstPlayBin2:playbin20/GstPlaySink:playsink0/GstBin:abin/GstBin:bin0/GstPulseSink:pulsesink0...
^CCaught interrupt -- handling interrupt.
Interrupt: Stopping pipeline ...
Execution ended after 16281646726 ns.
Setting pipeline to PAUSED ...
Setting pipeline to READY ...
Setting pipeline to NULL ...
Freeing pipeline ...
Comment 9 Colin Guthrie 2011-08-23 01:26:50 UTC
Yeah the cork event is never sent as module-cork-music-on-phone notices that the stream is already corked and thus it doesn't do any more. The reason it's corked in this case is due to the START_CORKED flag. However, at present, we cannot differentiate between the initial cork and one from later on. One option is to remove the START_CORKED flag after the initial uncork. That way if the stream is corked and the flag is set, we know to still act as if the stream is uncorked. This could actually be the simplest solution, but it would removed a degree of debug friendliness from the stream flags which is not good. As I said, I need to speak with Lennart to remind myself what we discussed!
Comment 10 Tanu Kaskinen 2011-08-23 10:13:11 UTC
(In reply to comment #9)
> Yeah the cork event is never sent as module-cork-music-on-phone notices that
> the stream is already corked and thus it doesn't do any more.

Why doesn't module-cork-music-on-phone send the cork request always, even if the stream is already corked? What possible harm could that do? If the request would be sent always, the client would at least have a chance to work around the shortcomings of the protocol.
Comment 11 Colin Guthrie 2011-08-23 10:27:32 UTC
(In reply to comment #10)
> (In reply to comment #9)
> > Yeah the cork event is never sent as module-cork-music-on-phone notices that
> > the stream is already corked and thus it doesn't do any more.
> 
> Why doesn't module-cork-music-on-phone send the cork request always, even if
> the stream is already corked? What possible harm could that do? If the request
> would be sent always, the client would at least have a chance to work around
> the shortcomings of the protocol.

Because the opposite would also have to be true - i.e. always sending the uncork request. If that is the case and the client does not implement cork request counting then you'd instantly get several clients incorrectly unpausing music players at the end of calls even if the music had not been playing when the call came in.

Not saying it's not a good idea generally, but it would have to be documented quite clearly.
Comment 12 Tanu Kaskinen 2011-08-23 10:49:08 UTC
(In reply to comment #11)
> Because the opposite would also have to be true - i.e. always sending the
> uncork request. If that is the case and the client does not implement cork
> request counting then you'd instantly get several clients incorrectly unpausing
> music players at the end of calls even if the music had not been playing when
> the call came in.
> 
> Not saying it's not a good idea generally, but it would have to be documented
> quite clearly.

Did Lennart really have some genius idea for how things could be fixed without requiring any extra effort from clients?
Comment 13 Himanshu Chug 2011-08-23 23:45:38 UTC
(In reply to comment #9)
> Yeah the cork event is never sent as module-cork-music-on-phone notices that
> the stream is already corked and thus it doesn't do any more. 

Just one clarification:

we are able to mute the "music" stream that means that the below code get executed and "if" condition is satisfied and this also means that CORK request is sent from the module using pa_sink_input_send_event(j, PA_STREAM_EVENT_REQUEST_CORK, NULL) function below?  but then I guess CORK request is getting dropped somewhere else outside the module ?

apply_cork()
{

....
..
        if (cork && !corked) {
            pa_hashmap_put(u->cork_state, j, PA_INT_TO_PTR(1));
            pa_sink_input_set_mute(j, TRUE, FALSE);
            pa_sink_input_send_event(j, PA_STREAM_EVENT_REQUEST_CORK, NULL);
        }

..
}
Comment 14 GitLab Migration User 2018-07-30 10:19:35 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/344.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.