Modify
Warning Please read the Ticket HowTo before creating or commenting on a ticket. Failure to do so may cause your ticket to be rejected or result in a slower response.

Opened 3 years ago

Closed 3 years ago

#9930 closed Patch - Bug Fix (fixed)

Distorted alsa-output on some soundcards

Reported by: mac20xx@… Owned by: jyavenard
Priority: minor Milestone: 0.24.1
Component: MythTV - Audio Output Version: 0.24.1
Severity: medium Keywords: distorted alsa output
Cc: Ticket locked: no

Description

libmyth/audiooutputalsa.cpp: period_size and buffer_size are not aligned correctly to alsa-frames. Depending on your soundcard this may cause distorted output.

In my case (Terratec EWX24/96 with ICE1712 chip) on every call of WriteAudio?() one sample (only left or right channel) was left out so that the channels where swapped after every call of WriteAudio?(). That has caused distorted output on alsa-output.

AudioOutputALSA::WriteAudio?():

added some error checking

AudioOutputALSA::SetParameters?():

new computation of period_size and buffer_size

Attachments (6)

audiooutputalsa.cpp.diff (2.0 KB) - added by mac <mac20xx@…> 3 years ago.
mythtv.log---0.24.1-Cambridge (36.7 KB) - added by mac <mac20xx@…> 3 years ago.
mythtv.log---0.24.1-Terratec (22.9 KB) - added by mac <mac20xx@…> 3 years ago.
mythtv.log---patched-Cambridge (30.1 KB) - added by mac <mac20xx@…> 3 years ago.
mythtv.log---patched-Terratec (29.0 KB) - added by mac <mac20xx@…> 3 years ago.
audio-scrambled.diff (643 bytes) - added by jyavenard 3 years ago.

Download all attachments as: .zip

Change History (19)

Changed 3 years ago by mac <mac20xx@…>

comment:1 follow-up: Changed 3 years ago by jyavenard

While I do not doubt that their is a problem...

I am not sure your interpretation of it is correct.

period_size and buffer_size are values returned by ALSA, it is not set to a random value pulled out of thin air.

The write_size is always fragment_size which is period_size * output_bytes_per_frame * 2 so by definition it is always a multiple of period_size.

So if period_size isn't a multiple of "alsa-frames" (as you call it) ; the issue is within ALSA, not myth...

Something that wouldn't surprise me as the type of audio corruption you describe never occurred with ALSA <= 1.0.22 and only occurs with recent version of alsa

comment:2 Changed 3 years ago by jyavenard

Your patch makes no sense..
10 + if ((size % (output_bytes_per_frame * channels)) != 0) {
11 + int size_needed = (size / (output_bytes_per_frame * channels)) * channels * output_bytes_per_frame;
12 + VBERROR(QString(LOC + "WriteAudio?: data not aligned to alsa-frame size"));
13 + VBERROR(QString(LOC + " size: %1, size needed: %2")
14 + .arg(size).arg(size_needed));
15 + }
16 +

output_bytes_per_frame is bytes_per_channel * channels ; a frame is made of samples for all channels. your testing a modulo of bytes_per_channels * channels * channels...

I think you're are confused between the meaning of a frame, fragment_size and period_size... As for a what a "alsa-frame" is, it's the first time I've ever heard of such term.

Have a read: http://www.alsa-project.org/main/index.php/FramesPeriods

Last edited 3 years ago by jyavenard (previous) (diff)

comment:3 in reply to: ↑ 1 Changed 3 years ago by mac <mac20xx@…>

The write_size is always fragment_size which is period_size * output_bytes_per_frame * 2 so by definition it is always a multiple of period_size.

As long as you're only multiplying you're right

So if period_size isn't a multiple of "alsa-frames" (as you call it) ; the issue is within ALSA, not myth...

I called them alsa-frames because they are not the same as frames in mythtv

I will switch back to mythtv 0.24.1 without patches to give you some debugging output to you.

Something that wouldn't surprise me as the type of audio corruption you describe never occurred with ALSA <= 1.0.22 and only occurs with recent version of alsa

I don't believe that this is an alsa related problem. Audacious always worked fine.

comment:4 follow-up: Changed 3 years ago by jyavenard

"I called them alsa-frames because they are not the same as frames in mythtv"

They are!

That's why they are called frames instead of samples.
frame = sample * channels (just like in ALSA)

So when you do:
channels * output_bytes_per_frame
you are actually doing:
channels * channels * sample.

I have no doubt that there could be a problem, but your analysis is wrong and should it fix anything it's a side effect of your incorrect calculations.

Changed 3 years ago by mac <mac20xx@…>

Changed 3 years ago by mac <mac20xx@…>

Changed 3 years ago by mac <mac20xx@…>

Changed 3 years ago by mac <mac20xx@…>

comment:5 in reply to: ↑ 4 Changed 3 years ago by mac <mac20xx@…>

Replying to jyavenard:

"I called them alsa-frames because they are not the same as frames in mythtv"

They are!

That's why they are called frames instead of samples.
frame = sample * channels (just like in ALSA)

So when you do:
channels * output_bytes_per_frame
you are actually doing:
channels * channels * sample.

I have no doubt that there could be a problem, but your analysis is wrong and should it fix anything it's a side effect of your incorrect calculations.

I' ve attached 4 logfiles to the original ticket created with mythfrontend -v audio,timestamp

Testing with the original code gave distorted output with 2 soundcards (Terratec EWX 24/96 and Cambridge DacMagic? via usb).

The reason for this is a wrong calculated "size" given to WriteAudio?()

Terratec EWX:
2011-07-22 10:33:09.617 WriteAudio?: Preparing 2622 bytes (655 frames)

data in the buffer 2622 bytes, written to soundcard 655*2*2 = 2620 bytes so one sample is missing.

Same for Cambridge DacMagic?:

2011-07-22 10:29:49.960 WriteAudio?: Preparing 8822 bytes (2205 frames)

data in the buffer 8822 bytes, written to soundcard 2205*2*2 = 8820 bytes so one sample is missing.

Because only one sample is missing after every call of WriteAudio?() the channels are switched.

I'm not a c-programmer so the patches might be done better but I think my analysis of the problem is ok.

comment:6 follow-ups: Changed 3 years ago by jyavenard

Can you try with the attached patch and let me know if you are still experiencing the problem...

Changed 3 years ago by jyavenard

comment:7 in reply to: ↑ 6 ; follow-up: Changed 3 years ago by mac <mac20xx@…>

Replying to jyavenard:

Can you try with the attached patch and let me know if you are still experiencing the problem...

I'm using the patch for several weeks and it's ok for me (see mythtv.log---patched-*). I've only tested with stereo configuration so someone should try it on surround systems.

comment:8 in reply to: ↑ 6 Changed 3 years ago by mac <mac20xx@…>

Sorry haven't seen your patch! I'll try it immediatly.

comment:9 follow-ups: Changed 3 years ago by jyavenard

There is something else in your log that is puzzling:

2011-07-22 10:02:47.387 ALSA: Hardware audio buffer cur: 256 need: 896 max allowed: 256
2011-07-22 10:02:47.387 ALSA, Error: Unable to sufficiently increase ALSA hardware buffer size - underruns are likely

That's to get 110kB of buffer, it shouldn't request 896kB of buffer, 256kB is plenty... another bug there...

comment:10 in reply to: ↑ 9 Changed 3 years ago by mac <mac20xx@…>

Replying to jyavenard:

There is something else in your log that is puzzling:

2011-07-22 10:02:47.387 ALSA: Hardware audio buffer cur: 256 need: 896 max allowed: 256
2011-07-22 10:02:47.387 ALSA, Error: Unable to sufficiently increase ALSA hardware buffer size - underruns are likely

That's to get 110kB of buffer, it shouldn't request 896kB of buffer, 256kB is plenty... another bug there...

I don't think that's a problem. Alsa first tries to get 500ms of buffer time and then tries to increase the buffer (echo >/proc/asound/...). If that fails mythtv throws the error message but I never had problems with buffer underruns. (The max. buffer time that could be allocated on the Terratec is about 140ms (16-bit stereo))

comment:11 in reply to: ↑ 7 Changed 3 years ago by mac <mac20xx@…>

Ok! Your patch works.

Terratec: 2011-07-22 14:06:14.503 WriteAudio?: Preparing 2620 bytes (655 frames)

Cambridge: 2011-07-22 14:15:36.282 WriteAudio?: Preparing 8820 bytes (2205 frames)

comment:12 in reply to: ↑ 9 Changed 3 years ago by mac <mac20xx@…>

Replying to jyavenard:

There is something else in your log that is puzzling:

2011-07-22 10:02:47.387 ALSA: Hardware audio buffer cur: 256 need: 896 max allowed: 256
2011-07-22 10:02:47.387 ALSA, Error: Unable to sufficiently increase ALSA hardware buffer size - underruns are likely

That's to get 110kB of buffer, it shouldn't request 896kB of buffer, 256kB is plenty... another bug there...

I don't believe that the Terratec has a buffer of 256k. I think alsa-lib returns the wrong size in /proc/asound/.../prealloc but this value is used to compute the needed buffer size.

int cur = pfile.readAll().trimmed().toInt();
int max = mfile.readAll().trimmed().toInt();

int size = ((int)(cur * (float)requested / (float)buffer_time)

/ 64 + 1) * 64;

VBAUDIO(QString("Hardware audio buffer cur: %1 need: %2 max allowed: %3")

.arg(cur).arg(size).arg(max));

If cur is reported incorrectly from alsa-lib then also size is computed wrong but that's really a alsa problem.

As stated above: your patch works!
Thanks a lot for your help.

comment:13 Changed 3 years ago by Github

  • Resolution set to fixed
  • Status changed from new to closed

Under some circumstances, using ALSA due to bad rounding, some audio samples would be lost.

Should the number of frames for period_size returned by ALSA be an odd-number, some samples would be lost.

Fix #9930.

Branch: master
Changeset: a56a7594102ee5fe05383d8fb68826efd8a6cf9c

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'new'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.