On 14 Jul 2017, at 14:27, Christophe de Dinechin <dinechin@xxxxxxxxxx> wrote:
>
> This looks better than my fix. Can’t ack right now (I did not build and test it yet), but that looks like a reasonable approach
>
>> On 14 Jul 2017, at 14:24, Pavel Grunt <pgrunt@xxxxxxxxxx> wrote:
>>
>> This also fixes the huge memore leak reported by Christophe.
>> Steps to reproduce:
>> 1. ./configure --disable-pulse --enable-gstaudio
>> 2. connect to a vm streaming video with no audio playing
>> 3. see the memory grow really fast
>>
>> It seems to be due to the fact that the "fake audio" channel updates the mmtime
>> in a way that schedules the frames to the future ((unsigned) 0 - delay) and then
>> gstreamer starts to queue the frames (they should be played later)
>>
>> Pavel
>>
>> On Fri, 2017-07-14 at 13:19 +0200, Pavel Grunt wrote:
>>> The fake channel has been introduced to get the audio volume by starting
>>> the gstreamer's audio pipeline and querring its volume info (see commit
>>> aa8d044417bbf60685f59163b874ecb4f157c3c9).
>>>
>>> Hovewer starting the pipeline updates the mmtime as a side effect. This
>>> may cause a (big) delay in displaying a video stream.
>>>
>>> Because the fake channel is only needed to get the volume info, make
>>> sure to not update the mm-time with it.
>>>
>>> Reported-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>>> ---
>>> src/spice-gstaudio.c | 11 +++++++----
>>> 1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/spice-gstaudio.c b/src/spice-gstaudio.c
>>> index 014c3a5..715f824 100644
>>> --- a/src/spice-gstaudio.c
>>> +++ b/src/spice-gstaudio.c
>>> @@ -38,6 +38,7 @@ struct stream {
>>> GstElement *sink;
>>> guint rate;
>>> guint channels;
>>> + gboolean fake; /* fake channel just for getting info about
>>> audio (volume) */
>>> };
>>>
>>> struct _SpiceGstaudioPrivate {
>>> @@ -264,6 +265,8 @@ static gboolean update_mmtime_timeout_cb(gpointer data)
>>> SpiceGstaudioPrivate *p = gstaudio->priv;
>>> GstQuery *q;
>>>
>>> + g_return_val_if_fail(!p->playback.fake, TRUE);
>>> +
>>> q = gst_query_new_latency();
>>> if (gst_element_query(p->playback.pipe, q)) {
>>> gboolean live;
>>> @@ -326,7 +329,7 @@ cleanup:
>>> if (p->playback.pipe)
>>> gst_element_set_state(p->playback.pipe, GST_STATE_PLAYING);
>>>
>>> - if (p->mmtime_id == 0) {
>>> + if (!p->playback.fake && p->mmtime_id == 0) {
>>> update_mmtime_timeout_cb(gstaudio);
>>> p->mmtime_id = g_timeout_add_seconds(1, update_mmtime_timeout_cb,
>>> gstaudio);
>>> }
>>> @@ -569,7 +572,6 @@ static gboolean
>>> spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
>>> GstElement *e;
>>> gboolean lmute;
>>> gdouble vol;
>>> - gboolean fake_channel = FALSE;
>>> GTask *task = G_TASK(res);
>>>
>>> g_return_val_if_fail(g_task_is_valid(task, audio), FALSE);
>>> @@ -584,9 +586,9 @@ static gboolean
>>> spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
>>>
>>> if (p->playback.sink == NULL || p->playback.channels == 0) {
>>> SPICE_DEBUG("PlaybackChannel not created yet, force start");
>>> + p->playback.fake = TRUE;
>>> /* In order to get system volume, we start the pipeline */
>>> playback_start(NULL, SPICE_AUDIO_FMT_S16, 2, 48000, audio);
>>> - fake_channel = TRUE;
>>> }
>>>
>>> if (GST_IS_BIN(p->playback.sink))
>>> @@ -604,9 +606,10 @@ static gboolean
>>> spice_gstaudio_get_playback_volume_info_finish(SpiceAudio *audio
>>> }
>>> g_object_unref(e);
>>>
>>> - if (fake_channel) {
>>> + if (p->playback.fake) {
>>> SPICE_DEBUG("Stop faked PlaybackChannel");
>>> playback_stop(SPICE_GSTAUDIO(audio));
>>> + p->playback.fake = FALSE;
>>> }
>>>
>>> if (mute != NULL) {
This fixes the issue I was seeing with a black screen.
Acked-by: Christophe de Dinechin <dinechin@xxxxxxxxxx>
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel