Ticket #839 (closed defect: fixed)

Opened 23 months ago

Last modified 22 months ago

Pulseaudio tries to rewind on ioplug devices

Reported by: mcarans Owned by: lennart
Milestone: 0.9.22 Component: module-alsa-*
Keywords: Cc: rans@…

Description

I have in .asoundrc:

 # Encode AC3 -> Directly on hardware
 pcm.Filter_A52Encode {
     type a52
     bitrate 448
     card NVidia
 }

 # Rate Converter to 48kHz, needed for Pulseaudio it seems
 pcm.Filter_RateConvert {
     type rate slave {
        pcm "Filter_A52Encode" rate 48000
     }
 }

and in default.pa:

 load-module module-alsa-sink device=Filter_RateConvert rate=48000 channels=6 tsched=0 sink_properties=device.description=SPDIF sink_name=SPDIF

Music plays through AC3 via SPDIF through Pulseaudio and Alsa, but when I close the application (eg. Rhythmbox music player), the SPDIF output "disappears".

I have attached a back trace: pulseaudiooutput_rateconverter.txt

I think it's here where the Filter_RateConvert sink gets unloaded for some reason:

 I: client.c: Freed 1 "Rhythmbox"
 I: protocol-native.c: Connection died.
 I: module.c: Unloading "module-alsa-sink" (index: #4).
 D: module-always-sink.c: Autoloading null-sink as no other sinks detected.

Is the sink being wrongly unloaded? Or is it correctly unloaded on Rhythmbox exit but wrong that it is not then reloaded afterwards?

Attachments

pulseaudiooutput_rateconverter.txt Download (45.9 KB) - added by mcarans 23 months ago.

Change History

Changed 23 months ago by mcarans

  Changed 23 months ago by mcarans

  • cc rans@… added

  Changed 23 months ago by mcarans

I added to default.pa: load-module module-suspend-on-idle timeout=0

and it seemed to prevent the sink unloading with Rhythmbox. SO I could open and close Rhythmbox and playing through SPDIF would work,

However, when I load Oolite that uses libsdl then the unloading still seems to occur (so Oolite produces no sound).

in reply to: ↑ description   Changed 23 months ago by tanuk

  • status changed from new to closed
  • resolution set to elsewhere
  • component changed from daemon to module-alsa-*

Replying to mcarans:

Is the sink being wrongly unloaded? Or is it correctly unloaded on Rhythmbox exit but wrong that it is not then reloaded afterwards?

From pulseaudiooutput_rateconverter.txt:

D: alsa-sink.c: Requested to rewind 55296 bytes.
D: alsa-sink.c: Limited to 55248 bytes.
D: alsa-sink.c: before: 4604
E: alsa-sink.c: snd_pcm_rewind() failed: Unknown error 4604
D: alsa-sink.c: process_rewind: Unknown error 4604
E: alsa-sink.c: process_rewind: Unknown error 4604

That error causes the alsa sink to quit. To be clear, the first error (E: alsa-sink.c: snd_pcm_rewind() failed: Unknown error 4604) is printed after calling snd_pcm_rewind unsuccessfully. After that pulseaudio calls try_recover, which repeats the error message in the beginning of the function, so the second error is really from the same call. The third error message is printed after trying to call snd_pcm_recover() inside try_recover. It is only when that snd_pcm_recover() call fails that the alsa sink gives up and quits - if snd_pcm_rewind() would fail, but snd_pcm_recover() would succeed, the sink would keep on going.

The bug is probably in the rate converter alsa plugin. I guess I can resolve this as "elsewhere".

  Changed 23 months ago by tanuk

Oh, I noticed only now that the magic number 4604 happens to be also the number of frames passed to snd_pcm_rewind() (it's that "before: 4604" line that reveals this). That indicates that snd_pcm_rewind() probably actually succeeds but it just manages to return the amount of frames as a negative number, so maybe there's just some sign error in the rewind implementation in the rate converter plugin.

follow-up: ↓ 6   Changed 23 months ago by mcarans

I forced Pulseaudio not to rewind using:

pa_sink_set_max_rewind(u->sink, 0);

This seemed to fix it.

timeout=0 is no longer needed in default.pa: default.pa: load-module module-suspend-on-idle # timeout=0

However, there is still a need for a rate converter in asoundrc: # Rate Converter to 48kHz, needed for Pulseaudio it seems pcm.Filter_RateConvert {

type rate slave {

pcm "Filter_A52Encode" rate 48000

}

}

Is there a proper way to implement the hack: pa_sink_set_max_rewind(u->sink, 0)?

in reply to: ↑ 5   Changed 23 months ago by tanuk

Replying to mcarans:

Is there a proper way to implement the hack: pa_sink_set_max_rewind(u->sink, 0)?

pa_sink_set_max_rewind(u->sink, 0) should be called only if we know that the device doesn't support rewinding. I'm not aware of a way to discover that.

Since you seem to be happy with recompiling pulseaudio and trying stuff, what if you replace this section in process_rewind() in src/modules/alsa/alsa-sink.c

        if ((out_frames = snd_pcm_rewind(u->pcm_handle, (snd_pcm_uframes_t) in_frames)) < 0) {
            pa_log("snd_pcm_rewind() failed: %s", pa_alsa_strerror((int) out_frames));
            if (try_recover(u, "process_rewind", out_frames) < 0)
                return -1;
            out_frames = 0;
        }

with this code

        if ((in_frames = PA_MIN(in_frames, snd_pcm_rewindable(u->pcm_handle))) < 0) {
            pa_log("snd_pcm_rewindable() failed: %s", pa_alsa_strerror((int) in_frames));
            if (try_recover(u, "process_rewind", in_frames) < 0)
                return -1;
            out_frames = 0;
        } else ((out_frames = snd_pcm_rewind(u->pcm_handle, (snd_pcm_uframes_t) in_frames)) < 0) {
            pa_log("snd_pcm_rewind() failed: %s", pa_alsa_strerror((int) out_frames));
            if (try_recover(u, "process_rewind", out_frames) < 0)
                return -1;
            out_frames = 0;
        }

That is, before calling snd_pcm_rewind(), limit in_frames to be at most whatever snd_pcm_rewindable() returns.

follow-up: ↓ 9   Changed 23 months ago by mcarans

That unfortunately caused a seg fault: D: memblockq.c: memblockq sanitized: maxlength=4194304, tlength=1698, base=1, prebuf=1478, minreq=221 maxrewind=0 I: protocol-native.c: Final latency 250.01 ms = 113.92 ms + 2*20.05 ms + 96.00 ms D: core-subscribe.c: Dropped redundant event due to change event. D: module-suspend-on-idle.c: Sink SPDIF becomes idle, timeout in 5 seconds. D: alsa-sink.c: Requested to rewind 55296 bytes. D: alsa-sink.c: Limited to 55272 bytes. D: alsa-sink.c: before: 4606 Segmentation fault

I tried doing something along the lines of:

size_t max_rewind = PA_MIN(u->hwbuf_size, snd_pcm_rewindable(u->pcm_handle)); pa_sink_set_max_rewind(u->sink, max_rewind);

But u->pcm_handle is not populated at that stage as far as I can tell.

  Changed 22 months ago by mcarans

Finally I got it all working including a52 Pulseaudio profiles solving the rewinding problem which caused the sink to unload!

These are the steps on Ubuntu Lucid 10.04 LTS:

1. Open from Pulse source code: src/modules/alsa/alsa-sink.c
2. Replace: pa_sink_set_max_rewind(u->sink, u->hwbuf_size);      with:
    if(strcmp(u->device_name, "a52") == 0) {
        pa_sink_set_max_rewind(u->sink, 0);
    } else {
        pa_sink_set_max_rewind(u->sink, u->hwbuf_size);
    }
3. Add Colin's patch:
             if (PA_UNLIKELY((sframes = snd_pcm_mmap_commit(u->pcm_handle, offset, frames)) < 0)) {

+                if (!after_avail && (int) sframes == -EAGAIN)
+                    break;
+
                 if ((r = try_recover(u, "snd_pcm_mmap_commit", (int) sframes)) == 0)
                     continue;
4. Compile and install Pulseaudio with udev support: make and sudo make install 
5. Edit /usr/local/share/pulseaudio/alsa-mixer/profile-sets/default.conf: change a52:%f to a52
6. Copy /etc/pulse/daemon.conf to ~/.pulse/daemon.conf and ensure it has uncommented: default-sample-rate = 48000
7.  Edit ~/.asoundrc
8.  Put in it:
# Encode AC3 -> Directly on hardware
pcm.Filter_A52Encode {
    type a52
    bitrate 448
    channels 6
    card NVidia
}

# Rate Converter to 48kHz, needed for some applications
pcm.a52 {
    type rate
    slave {
        pcm "Filter_A52Encode"
        rate 48000
    }
}
9. Change NVidia to your card's name from: cat /proc/asound/cards
10. sudo alsa reload
11. Make sure you do not have a ~/.pulse/default.pa or if you do, that it is using load-module module-udev-detect to load Alsa sinks not load-module module-alsa-sink ... 
  
12. pulseaudio -k
13. pulseaudio -D

in reply to: ↑ 7   Changed 22 months ago by tanuk

Replying to mcarans:

That unfortunately caused a seg fault:

Would you like to get a backtrace of it? I can't see any errors in the code I pasted, so I suspect the crash happens in snd_pcm_rewindable(), but that needs to be verified.

It's ok if you don't want to bother with the backtrace - I will probably try to reproduce this myself this week, because to me it looks like a bug to call snd_pcm_rewind() without first calling snd_pcm_rewindable(), so I'd like to get the patch merged, and to achieve that I need to test the patch first.

  Changed 22 months ago by tanuk

  • status changed from closed to reopened
  • resolution elsewhere deleted
  • summary changed from Sink wrongly unloaded or not reloaded to Pulseaudio tries to rewind on ioplug devices

Reopening the bug, and changing the summary. Rationale below:

Quoting Jaroslav Kysela[1]:

"The rewind/forward implementation in ioplug is actually broken, because it moves only internal pointer without notification to real external plugin."

So pulseaudio shouldn't try to rewind when it's using an ioplug device. I don't know whether it's possible to query alsa about the device type - maybe it's possible to ask if the device is an ioplug device, in which case we could set the sink's max rewind size to zero.

Regardless of whether it's possible to query the device type, calling snd_pcm_rewindable() before calling snd_pcm_rewind() should fix this problem - presumably ioplug devices will always return zero from snd_pcm_rewindable(). If that's not the case, then that will need to be fixed in alsa.

[1]  http://thread.gmane.org/gmane.linux.alsa.devel/52773

  Changed 22 months ago by mcarans

The only error in the code you pasted was the else should say else if.

Yes, the crash is in snd_pcm_rewindable(). I believe it's because u->pcm_handle for the sink is null (it isn't for other hardware sinks in my setup).

There's an enum for PCM types:  http://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#g60df675e39a8c0da3488bc06b1fe34ff

const char * snd_pcm_type_name (snd_pcm_type_t type): get name of PCM type might be helpful

Unfortunately I couldn't find a function that actually returns you a snd_pcm_type_t, but I'm sure one must exist.

follow-up: ↓ 13   Changed 22 months ago by coling

Thanks for working on this. FWIW, your use of a single "a52" device (without the card number after) is not necessary. Even tho' you do a quick (aka hacky!) check for the a52 device in the code, you could instead check to see if the string starts with "a52:" instead and put in the card argument in your a52 .asoundrc definition. That would allow this approach to work with >1 card at a time which is nice.

That said, hopefully tanuk's approach will make the hacky check in the code unnecessary. Looking forward to getting this working :D

in reply to: ↑ 12   Changed 22 months ago by tanuk

Replying to coling:

Thanks for working on this. FWIW, your use of a single "a52" device (without the card number after) is not necessary. Even tho' you do a quick (aka hacky!) check for the a52 device in the code, you could instead check to see if the string starts with "a52:" instead and put in the card argument in your a52 .asoundrc definition. That would allow this approach to work with >1 card at a time which is nice.

Do you have an alsa configuration snippet for a52 that accepts the card index? Pierre provided one example in [1], but as far as I understand it, that definition throws the card index away and still uses whatever card the a52 plugin chooses by default. Therefore, it's not good in case of multiple cards.

[1]  https://tango.0pointer.de/pipermail/pulseaudio-discuss/2010-July/007503.html

  Changed 22 months ago by mcarans

Colin, pl bossart in Pulseaudio email list discussion came up with improvements:

NAK. This does not work for me. You need to rewind also in the thread context:

diff --git a/src/modules/alsa/alsa-sink.c b/src/modules/alsa/alsa-sink.c
index 2995c3c..4059a9b 100644
--- a/src/modules/alsa/alsa-sink.c
+++ b/src/modules/alsa/alsa-sink.c
@@ -953,7 +953,12 @@ static int update_sw_params(struct userdata *u) {
    }

    pa_sink_set_max_request_within_thread(u->sink, u->hwbuf_size -
u->hwbuf_unused);
- pa_sink_set_max_rewind_within_thread(u->sink, u->hwbuf_size);
+ if (pa_alsa_pcm_is_hw(u->pcm_handle))
+ pa_sink_set_max_rewind_within_thread(u->sink, u->hwbuf_size);
+ else {
+ pa_log("Disabling rewind_within_thread for device %s", u->device_name);
+ pa_sink_set_max_rewind_within_thread(u->sink, 0);
+ }

    return 0;
}
@@ -1906,7 +1911,12 @@ pa_sink *pa_alsa_sink_new(pa_module *m,
pa_modargs *ma, const char*driver, pa_ca
                (double) pa_bytes_to_usec(u->hwbuf_size, &ss) /
PA_USEC_PER_MSEC);

    pa_sink_set_max_request(u->sink, u->hwbuf_size);
- pa_sink_set_max_rewind(u->sink, u->hwbuf_size);
+ if (pa_alsa_pcm_is_hw(u->pcm_handle))
+ pa_sink_set_max_rewind(u->sink, u->hwbuf_size);
+ else {
+ pa_log("Disabling rewind for device %s", u->device_name);
+ pa_sink_set_max_rewind(u->sink, 0);
+ }

    if (u->use_tsched) {
        u->tsched_watermark =
pa_usec_to_bytes_round_up(pa_bytes_to_usec_round_up(tsched_watermark,
&requested_ss), &u->sink->sample_spec);

pa_pcm_is_hw detects a52 but allows rewind for iec958. Looks like a
better fix than a hard-coded string compare.
"

I didn't have a line with pa_sink_set_max_rewind_within_thread but I added it anyway with no ill effect. Probably my Pulseaudio is older.

He also came up with a great way to avoid editing /usr/local/share/pulseaudio/alsa-mixer/profile-sets/default.conf

# Rate Converter to 48kHz, needed for some applications
pcm.a52 {
    @args.0 {
        type integer
    }
    type rate
    slave {
        pcm "Filter_A52Encode"
        rate 48000
    }
}

The args.0 will then allow a52:%f to match a52:0.

Now all we need is a way to avoid specifying "card NVidia"? "card 0" would work for the majority of users including me, but it would be nice to have something that will work if the card is not 0.

follow-up: ↓ 16   Changed 22 months ago by coling

Yeah I read the mailing list after the bug list :D

You shouldn't need to specify either card NVidia or card 0. Just use the argument passed in and pass it through to the slave. It's just about cooking up the right .asoundrc syntax. Something like (untested):

pcm.Filter_A52Encode {
    @args [ CARD ]
    @args.CARD {
        type integer
        default 0
    }
    type a52
    bitrate 448
    channels 6
    card $CARD
}


pcm.a52 {
    @args [ CARD ]
    @args.CARD {
        type integer
        default 0
    }
    type rate
    slave {
        pcm "Filter_A52Encode"
        card $CARD
        rate 48000
    }
}

in reply to: ↑ 15   Changed 22 months ago by mcarans

Replying to coling:

Yeah I read the mailing list after the bug list :D You shouldn't need to specify either card NVidia or card 0. Just use the argument passed in and pass it through to the slave. It's just about cooking up the right .asoundrc syntax.

Alsa asoundrc lesson for me:

1. Does CARD get populated by Pulseaudio?

2. If so, is that when udev tries to open a52:0, a52:1 etc.?

3. If not, how does CARD get populated?

  Changed 22 months ago by coling

I'm not really sure but I think the number arguments (e.g. args.0) are mapped to the named arguments, so args.CARD will be the same as args.0 and $CARD = $0, but I really don't know for sure... I'm only really going on what I managed to scrape together from reading the files in /usr/share/alsa :p

  Changed 22 months ago by coling

  • status changed from reopened to closed
  • resolution set to fixed

So this is fixed by [cb55b00]

  Changed 22 months ago by coling

  • milestone set to 0.9.22
Note: See TracTickets for help on using tickets.