Opened 12 years ago

Closed 12 years ago

#4764 closed patch (fixed)

alsa buffer problem introduced in 15893

Reported by: manwe Owned by: danielk
Priority: minor Milestone: 0.22
Component: mythtv Version: head
Severity: medium Keywords: alsa
Cc: nettibug@… Ticket locked: no

Description

At the time of this report HEAD (r16234) breaks my audio setup. r15728 worked just fine. I'm using alsa:default as audio output and I'm running Fedora 8 with pulseaudio. I hear sound for 2-10 seconds while watching a recording (or anything to do with sound in mythtv). I've tracked the problem to this change: libs/libmyth/audiooutputalsa.cpp lines: 92-94

15893 danielk fragment_size = 15893 danielk (audio_bits * audio_channels * audio_samplerate) / (8*30); 15893 danielk buffer_time = 100000;

No error are reported...

Changing it back returns fully functional audio with my setup.

Attachments (6)

15728-alsapatch (590 bytes) - added by anonymous 12 years ago.
patch to return old buffer and fragment size
t4764.diff (590 bytes) - added by Janne Grunau 12 years ago.
using suggested values by Mark Spieth
mythtv_ac3.54.patch.bz2 (1.6 KB) - added by Mark Spieth 12 years ago.
mythtv_mm_restrict.1.patch (4.4 KB) - added by Mark Spieth 12 years ago.
myth-mono-audio.log (203.1 KB) - added by laga+mythtv@… 12 years ago.
mythfrontend -v important,general,audio,playback for mono ac3 stream
mythtv_ac3.54-v2.patch (4.1 KB) - added by danielk 12 years ago.
updated patch

Download all attachments as: .zip

Change History (32)

Changed 12 years ago by anonymous

Attachment: 15728-alsapatch added

patch to return old buffer and fragment size

comment:1 Changed 12 years ago by manwe

Another test confirms that it was the buffer_time variable buffer_time = 100000;
setting it back to 500000 resolves the problem

comment:2 Changed 12 years ago by visit0r

This patch also fixes the problem I reported in #4788

comment:3 Changed 12 years ago by curtis@…

FYI, this patch fixed the problem I had with 32kHz audio sounding like it was underwater.

comment:4 Changed 12 years ago by Janne Grunau

#4711 is a duplicate of #4764

Changed 12 years ago by Janne Grunau

Attachment: t4764.diff added

using suggested values by Mark Spieth

comment:5 Changed 12 years ago by Janne Grunau

t4764.diff as suggested by Mark Spieth fixes the distortion for the sample in #4711

comment:6 Changed 12 years ago by manwe

Is there a reason why buffer_time should be smaller? I have not yet tested t4764.diff, but I will as soon as possible. Though 128000 (=32000*4) is still quite close to 100000 (which was too small).

comment:7 in reply to:  6 ; Changed 12 years ago by anonymous

Replying to manwe:

Is there a reason why buffer_time should be smaller? I have not yet tested t4764.diff, but I will as soon as possible. Though 128000 (=32000*4) is still quite close to 100000 (which was too small).

Digital spdif out doesn't work for me until i changed:

buffer_time = 32000 * 4; in usec

to

buffer_time = 32000 * 16; in usec

Even a setting of 8 was to small.

comment:8 Changed 12 years ago by danielk

Milestone: unknown0.21
Priority: minorcritical

We should try to fix this before 0.21, even if the fix is just to make the buffer insanely large...

comment:9 in reply to:  7 ; Changed 12 years ago by anonymous

Replying to anonymous:

Replying to manwe:

Is there a reason why buffer_time should be smaller? I have not yet tested t4764.diff, but I will as soon as possible. Though 128000 (=32000*4) is still quite close to 100000 (which was too small).

Digital spdif out doesn't work for me until i changed:

buffer_time = 32000 * 4; in usec

to

buffer_time = 32000 * 16; in usec

Even a setting of 8 was to small.

For me normal playback worked with 32000*4, but skiping repetitively loses audio. Doubling it (32000*8) was enough (at least with quick tests) to eliminate that problem.

32000*16=512000 is close to the original 500000, so what is the reasoning behind making it smaller?

comment:10 in reply to:  9 Changed 12 years ago by manwe

Replying to anonymous:

For me normal playback worked with 32000*4, but skiping repetitively loses audio. Doubling it (32000*8) was enough (at least with quick tests) to eliminate that problem.

32000*16=512000 is close to the original 500000, so what is the reasoning behind making it smaller?

me = manwe

comment:11 Changed 12 years ago by Mark Spieth

ok no choice but to set it back to 500ms.

Impact is pause takes 1/2 sec before it stops, which is what I wanted to mitigate. 128 works fine on analog on my machines (xbox and via8235).

perhaps in 22 a tie-in to aggressive audio buffering config option.

comment:12 Changed 12 years ago by manwe

I was expecting something like that. Because this seems to affect setups with spdif most (mine also), it's a good idea to tie it with a config options as you say.

What I'm wondering is curtises problem and has he tried modifying only buffer_time, because my original patch reverted fragment_size also. (For me the only problem was buffer_time as I said in some of my posts)

comment:13 in reply to:  7 Changed 12 years ago by anonymous

Replying to anonymous:

Replying to manwe:

Is there a reason why buffer_time should be smaller? I have not yet tested t4764.diff, but I will as soon as possible. Though 128000 (=32000*4) is still quite close to 100000 (which was too small).

Digital spdif out doesn't work for me until i changed:

buffer_time = 32000 * 4; in usec

to

buffer_time = 32000 * 16; in usec

Even a setting of 8 was to small.

I did some more testing between 8 and 16. The smallest setting that digital spdif out worked was 11.

comment:14 Changed 12 years ago by danielk

(In [16461]) Refs #4764. Reverts dynamic buffer sizing from Mark Spieth's audio patch.

comment:15 Changed 12 years ago by danielk

Milestone: 0.210.22
Priority: criticalminor

It would be nice to get the dynamic buffer sizing working, it appears that some floor is needed for low bit-rate streams. But [16461] just reverted to the large buffer and static fragment size we were using before, so that this issue can be pushed off to 0.22.

Changed 12 years ago by Mark Spieth

Attachment: mythtv_ac3.54.patch.bz2 added

Changed 12 years ago by Mark Spieth

Attachment: mythtv_mm_restrict.1.patch added

comment:16 Changed 12 years ago by Mark Spieth

did some investigating over the weekend and I think I have a better handle on things. One of my updates cvhanged my deinterlacing settings to greedy (on a TV!) and stopped things from working on my xbox. took me a while to solve that but I did find that fragment and periodtime and buffertime dont have to be related.

so the solution is to choose fragment as 6144 for 2ch (scaled for 6ch) periodtime as something low (I chose 25ms) and buffer time something large enough. From other posts I chose 16*25 which will generally alloc all of the 64K that (my) alsa uses as max.

2 patches are attached as mythmusic was impacted as well. The 2sec limit on AddSample? has been removed as it seemed to impact some things and a separate interface and test was put in mythmusic. benefits are that mm changes faster as the limit has been set at 500ms and videos can have a bigger audiosync adjustment.

please test and see if it works in the various modes (esp. spdif)

comment:17 in reply to:  16 Changed 12 years ago by anonymous

Replying to markspieth:

These patches work for me in both mythtv and mythmusic using digital spdif out. Tried. sdtv 720x480 mpeg2 48khz audio, hdtv both 720p and 1080i and in mythmusic played 44.1 khz 256k and 192k mp3's all without any problems. (also played dvd's) Where i had problem's with the previous t4764.diff patch.

comment:18 Changed 12 years ago by danielk

Owner: changed from Isaac Richards to danielk
Status: newassigned
Type: defectpatch

I'll review & test these for regressions tonight. It wold be nice if more people tested though.

comment:19 Changed 12 years ago by Matthew Wire <devel@…>

AC3/dts passthru still not working for me here unfortunately. Works fine as long as passthru is disabled (though obviously without the ac3/dts soundtrack). Logs (-v audio,playback) don't seem to show anything unusual, other than: 2008-03-12 00:34:08.721 AFD: Warning, audio codec 0x71bf740 id(AC3) type (Audio) already open, leaving it alone. 2008-03-12 00:34:08.721 AFD: codec AC3 has 2 channels 2008-03-12 00:34:08.721 AFD: Warning, audio codec 0x5db2070 id(AC3) type (Audio) already open, leaving it alone. 2008-03-12 00:34:08.721 AFD: codec AC3 has 2 channels 2008-03-12 00:34:08.721 AFD: Warning, audio codec 0x5db2450 id(AC3) type (Audio) already open, leaving it alone. 2008-03-12 00:34:08.721 AFD: codec AC3 has 2 channels 2008-03-12 00:34:08.721 AFD: Warning, audio codec 0x5dce5c0 id(AC3) type (Audio) already open, leaving it alone. 2008-03-12 00:34:08.721 AFD: codec AC3 has 2 channels 2008-03-12 00:34:08.721 AFD: Warning, audio codec 0x53ef860 id(AC3) type (Audio) already open, leaving it alone. 2008-03-12 00:34:08.722 AFD: codec AC3 has 6 channels

comment:20 Changed 12 years ago by skamithi

latest patch from mark doesn't cause any problems for me. have spdif with ac3/dts passthru enabled.

comment:21 Changed 12 years ago by manwe

Current HEAD (r16554) works fine for me without any patches, thank you.

I'm on a process of testing marks patch.. so far sound seems to work fine with pulseaudio (mythtv with alsa), x86_64 system with spdif connection to amplifier. I'm not using music at the moment, so I'll be testing live tv and recordings and video only.

comment:22 Changed 12 years ago by laga+mythtv@…

Playback of mono AC3 streams doesn't work for me. I've got some DVDs with old shows where the audio track only has one channel and all i get is very tinny audio. I've tried to apply Mark Spieth's mythtv_ac3.54.patch.bz2 but that didn't fix the problem. I'll attach a -v important,general,audio,playback log I created without having mythtv_ac3.54.patch.bz2 applied.

Changed 12 years ago by laga+mythtv@…

Attachment: myth-mono-audio.log added

mythfrontend -v important,general,audio,playback for mono ac3 stream

comment:23 Changed 12 years ago by danielk

(In [16723]) Fixes #4981. AudioOutput? cleanup.

Some const correctness plus simplifying the constructors and Reconfigure so new optional audio paramameters can be added without needing to touch every audio file.

Refs #4764. I wanted to fix this after trying to fix some const problems in Steve Adeff's patch and running into a bunch of the that went all the way back to AudioOutput? and AudioOutputBase?.

This has been tested with ALSA/OSS/OSX/MINGW audio and compiles on all three platforms.

This does require a "make distclean" and rebuilding the plugins as the Audio API is slightly changed.

comment:24 in reply to:  16 Changed 12 years ago by Neil Garratt <ngarratt@…>

Replying to markspieth:

so the solution is to choose fragment as 6144 for 2ch (scaled for 6ch) periodtime as something low (I chose 25ms) and buffer time something large enough. From other posts I chose 16*25 which will generally alloc all of the 64K that (my) alsa uses as max.

please test and see if it works in the various modes (esp. spdif)

A buffer_time of 400000 was the lowest value I found usable previously, to this fixes it for me. I still can't timestretch AC3 material though, because it switches from the passthru ALSA device to the default ALSA device.

Changed 12 years ago by danielk

Attachment: mythtv_ac3.54-v2.patch added

updated patch

comment:25 Changed 12 years ago by Matthew Wire <devel@…>

Just a note to say ac3 over spdif is still broken for me with this latest patch.

comment:26 Changed 12 years ago by danielk

Resolution: fixed
Status: assignedclosed

(In [17251]) Fixes #4764. Refs #5313. Fixes ALSA output buffer sizing problem.

Note: See TracTickets for help on using tickets.