Periodical, short audio artifacts in one the channels when upmixing to 5.1

[blazej.lewcio@…]

When using upmixing to 5.1, I get a periodical audio artifacts in one of the left channels. It appears approx. every 16 s, and takes approx. 1 s (or less). It sounds like a mix of stuttering and oscillation (short circuit) between the central and affected speaker. It seems to be independent from the content. It appear whether I watch speech only (news) or speech with music background (movies).

It happens only with Live TV, in my case DVB-T. It does not happen with MythVideo?. Also, I don't get the artifacts after starting Live TV directly, but first when I switch the channels. I suspect, it can be reproduced when I switch the channel and than go back to the initial channel again.

I use #182496.

[JYA]

What version of myth are you using?

What's 182496

try latest master (0.25) and report..

[blazej.lewcio@…]

It is the commit signature from the master branch (Commit 18249695bed199c3003a945358e5f2564954b2dc, from Mon Feb 27 16:02:26).

[JYA]

Please provide a tad more detail.. I'm not a mind reader

Audio card type Audio settings used (standard and advanced) Audio format being played Provide a sample where you can reproduce the issue

And finally the output of playback with mythfrontend started with -v audio

[blazej.lewcio@…]

Affects both, "good" and "best" upmixing.

The input in my case is MP2, stereo.

My soundcard:

0 [Intel ]: HDA-Intel - HDA Intel

HDA Intel at 0xf9ff8000 irq 44

, and I use IEC958.

Log is attached. The artifacts could be perceived several times after the last channel change, but not before. The last change was switching back.

I'm not able to capture a sample of the artifact with simple Mythtv recording. The recordings are just fine. If there is a way to capture output signal to the speakers, I'd be happy to do this.

[blazej.lewcio@…]

Additional info: I can reproduce it by seeking in a video too. Thus, affects not only LiveTV but also (stereo-only) videos. Until now, not observed with pure 5.1 content (, but extensively tested).

Additional log extract from "-v all" is now attached. Information when the distortion was observed is included in the file. I don't get any log output correlated with the distortion when I use "-v audio" only.

[Mark Spieth]

Probably needs a reset method in freesurround to clear the fft mixing buffer on audio stream transitions.

[JYA]

Mark, the upmixer is destroyed/recreated whenever the audio format has changed. I would assume this would clear any internal buffer used by the upmixer.

However, if you would mind crafting a patch for that, it would be very much appreciated.

[Mark Spieth]

untested patch to clear audio state

[Mark Spieth]

added a patch (untested) to clear audio state on Reset() which happens after seek.

It appears audio is not destroyed under seek circumstances.

I expect this to work, but its a good idea in any case.

[JYA]

but for the problem to occur as the OP described, that would assume that a reset is occurring every 16s, which is unlikely to ever happen

[JYA]

blazej.lewcio@…, can you try the provided patch and report?

Thanks

[blazej.lewcio@…]

sure, I'll try it this afternoon (CET)

[blazej.lewcio@…]

Unfortunately, the patch does not fix the problem.

Additional info: If the sound starts to be distorted and I toggle off/on the upmixer, the upmixed sound works fine again, but if I then seek in the stream again, the problem returns.

[blazej.lewcio@…]

Additional info: the distortion clearly happens when the if condition at line 1246 (in my code) in libmyth/audio/audiooutputbase.cpp is entered.

 1243         bdFrames = (kAudioRingBufferSize - org_waud) / bpf;
 1244         printf("BufSize=%d, org_waud=%d, bpf=%d\n", kAudioRingBufferSize, org_waud, bpf);
 1245 
 1246         if (bdFrames < nFrames)
 1247         {
 1248             puts("       xxxx");
 1249             upmixer->receiveFrames((float *)(WPOS), bdFrames);
 1250             nFrames -= bdFrames;
 1251             org_waud = 0;
 1252         } 

[JYA]

The bug could be on how the upmixer handles when its buffer is completely full...

but certainly now how audiooutputbase calls it

Having said that, this conditions occurs several times a minute, and I've never seen the problem before, and I haven't had any reports of the problem before.

We did have a similar issue and it turned out to be an ALSA issue when using a particular audio buffer size.

To make sure that it's not what is occurring here, can you start the frontend with: ALSABufferOverride=200 mythfrontend -v audio

by default we use a value of 500ms, but this has caused problem for some.

[blazej.lewcio@…]

same problem with 200

[JYA]

Replying to blazej.lewcio@…:

Additional info: the distortion clearly happens when the if condition at line 1246 (in my code) in libmyth/audio/audiooutputbase.cpp is entered.

 1243         bdFrames = (kAudioRingBufferSize - org_waud) / bpf;
 1244         printf("BufSize=%d, org_waud=%d, bpf=%d\n", kAudioRingBufferSize, org_waud, bpf);
 1245 
 1246         if (bdFrames < nFrames)
 1247         {
 1248             puts("       xxxx");
 1249             upmixer->receiveFrames((float *)(WPOS), bdFrames);
 1250             nFrames -= bdFrames;
 1251             org_waud = 0;
 1252         } 

so you are saying that whenever the distortion occurs it coincides with the xxxx being printed ?

Can you reproduce the problem changing the quality of the upmixer to low in the settings?

[JYA]

oh, and can you post the output with the extra log you added ?

Thanks

[blazej.lewcio@…]

for all the last tests I used the "good" upmixer (out of two: good and best I can choose, low as such is not in the list). Log extract of the distortion moment is attached.

[JYA]

Ok.

And what about then if using the "Best" quality ?

[JYA]

With you log it doesn't have any timestamp, so it's impossible to tell what...

Replace your printf with: VBAUDIO(QString("BufSize?=%1, org_waud=%2, bpf=%3").arg(kAudioRingBufferSize).arg(org_waud).arg(bpf));

and your puts with: VBAUDIO(QString("Frames = %1, bdFrames = %2, nFrames = %3").arg(frames).arg(bdFrames).arg(nFrames));

[Mark Spieth]

This is the interesting bit

2012-03-10 12:22:10.329782 I  AO: BufSize=3072000, org_waud=1032192, bpf=24
2012-03-10 12:22:10.361386 I  AO: Pause 1
2012-03-10 12:22:10.363426 I  AO: OutputAudioLoop: audio paused
2012-03-10 12:22:13.075240 W  RingBuf(/home/mythtv/Recordings/18403_20120310122212.mpg): Not starting read ahead thread, already running
2012-03-10 12:22:13.136754 N  Player(0): Forcing decode extra audio option on (Video method requires it).
2012-03-10 12:22:13.754258 I  AFD: Opened codec 0xffffffffa80cfb40, id(MPEG2VIDEO) type(Video)
2012-03-10 12:22:13.754274 I  AFD: codec MP2 has 2 channels
2012-03-10 12:22:13.754310 I  AFD: Opened codec 0xffffffffa432ef10, id(MP2) type(Audio)
2012-03-10 12:22:13.754340 I  AFD: Audio Track #1 is A/V stream #2 and has 2 channels in the German language(6776178).
2012-03-10 12:22:13.754388 I  AFD: Selected track 1: German MP2 2ch (A/V Stream #2)
2012-03-10 12:22:13.754410 I  AFD: Initializing audio parms from audio track #1
2012-03-10 12:22:13.754441 I  AFD: Audio format changed
                        from id(NONE)     -1Hz -1ch -1bps     (profile 0) to id( MP2)  48000Hz  2ch 16bps     (profile 0)
2012-03-10 12:22:13.754458 I  AO: Needs upmix from 2 -> 6 channels
2012-03-10 12:22:13.754465 I  AO: Reconfigure(): No change -> exiting
2012-03-10 12:22:13.758942 N  AFD: Resetting byte context eof (livetv 1 was eof 0)
2012-03-10 12:22:13.949832 I  AO: BufSize=3072000, org_waud=1040384, bpf=24

notice that org_waud diff across the no change reconfigure is not a multiple of 24 (6*4)

(1040384-1032192)/24 = 341.333

not sure why yet. Unexpected.

In this case this would rotate the channels by 2 putting L,R into LS,RS or C,LFE, etc.

[JYA]

this is a very interesting find...

[JYA]

Great.. For some reason I didn't receive notifications that this log had been posted...

At some stage in the log, everything get shifted slightly... it's not just when it reset the circular buffer

[Mark Spieth]

i did not get notified either. attach file does not notify.

looks like 2 problems

encoder is in use. needs a clear during Reset (like the other modules) also in Reset() waud = raud may not preserve bpf boundary

first one is easy. have patch already.

2nd one is unclear ATM for what is thread safe.

[JYA]

Now that you mention that the AC3 encoder is in use. It all makes sense. And you're looking at a non-issue

For a start, the encoder is re-created during each reconfigure.. (Actually looks like there is a potential leak, the encoder isn't deleted if it already exists...)

2nd, doing a reset of the encoder is useless. It will always exhaust all the data it has been provided after it exits. so by the time it's finished processing the data given in AddFrames?, it is always empty

3rd: It's perfectly okay for org_waud to not be a multiple of bytesperframe. The audio is upmixed within the ringbuffer only as temporary storage. It is immediately compressed as AC3 and bytesperframe there isn't 24 anymore, but 4. You'll find that all those values are multiples of 4 as expected

[Mark Spieth]

it does explain 16 sec audio glitch period as the rate is now right @ 48k*4bytes * 3072000.

1. encoder not recreated if no change is detected AFAICT.

2. it will output old data for a short period before new data is output after stream change. proc state clear in reset is still advisable but not a (the) glitch generator.

3. encoder->Encode is called with bdiff (byte not frame multiple) which is not aligned to 6ch bpf which means it will absorb a fractional unset/uninited sample at buffer wrap time and then shift/rotate channel presentation to the encoder. fix waud reset and this will go away irrespective of the output bpf or fix incomplete buffer wrap condition in Encode wrap part.

Also I did extensive tests with audio out 6ch and I couldn't get it to fail in the way described.

[Mark Spieth]

updated patch

[Mark Spieth]

added patch for notify purposes.

[JYA]

while it does make sense to reset, in effect it's not necessary as none of the encoder will effectively buffer any data

I am fairly certain it wouldn't fix the issue the OP is seeing...

If you actually had a closer look on what it is actually doing, it actually works that way: org_waud = waud = next position to write if (upmixing) {

upmix and copy into org_waud and update it update number_of_frames_added

}

if (encoder) {

go back to beginning of new audio added: org_waud = waud encode number_of_frames_added

}

so where the upmixer write its data is irrelevant, the encoder will start from where the upmixer started. As the AC3 encoder will always produce a smaller output, there's no overlap of any kind.

[Mark Spieth]

I think you misunderstand. Another way would be like this

int bdFrames = bdiff / bpf;
encoder->Encode(WPOS, bdFrames * bpf, processing ? FORMAT_FLT : format);
to_get = encoder->Encode(ABUF, len - bdFrames * bpf,

however I think there are other cases and the proposed patch just handles everything intrinsically due to raud = 0 in Reset()

[JYA]

even if you do set raud to 0 during reset. it doesn't explain why corruption would occur every 16s... because reset isn't called every 16s.

All what you patch does is prevent old audio to be heard right after a reset. Which isn't what the OP is experiencing

[JYA]

Extra verbosity

[JYA]

blazej.lewcio@…, can you try the small patch above and post the output?

I just want to see if we ever get into the case where the audio data doesn't fit precisely the buffer..

Which could explain the corruption that you are seeing...

[JYA]

Mark, after looking further into this, while setting raud = 0 would reduce the issue from occurring, it doesn't guarantee that it won't happen. Plus there's the issue that due to a race condition with the write audio thread that raud wouldn't be set to 0 anyway (in the log above, you can clearly see that it's precisely what's occurring, actually_paused is set to 1 after Reset has been called).

So the main issue is that all the data written prior calling the AC3 encoder do not exactly fit into the ringbuffer and everything would get shifted..

I don't know how we could fix this without either changing on the fly the length of the ringbuffer (and then you have threading issues), or by using a separate buffer for performing the upmixing...

[Mark Spieth]

My understanding is that Reset which I care about is called when unpause occurs from PauseUntilBuffered? which would do the correct thing (in this case). Which is why I put in the safety of only doing it when paused.

I still think it will work the way I think.

If its not, handle it asyncly in the output thread with a flag set in Reset() to reset raud and waud there.

[JYA]

I'm actually thinking of doing it the following way:

make the audiobuffer 32 bytes bigger than what it currently is. Then stop using the constant kAudioRingBufferSize to determine the length of the buffer, instead use a dynamic variable. When doing a reset, the length of the audio buffer would actually be changed so the length between raud (which is now the new waud) and kAudioRingBufferSize is now a modulo of bpf. That way there is no thread/locking issue, and we will always be able to fit everything to the end of the buffer.

Doing so would actually remove the requirement of having such a big audio buffer, simply because it needs to be a multiple of all the possible bpf values

[JYA]

And we still have the issues that the gap between waud and the end of the audio buffer must be a multiple of bpf, which isn't guarantee coming out of the encoder. Although, this seems unlikely to occur often as much more people would have reported the audio corruption earlier

[Github]

Fully clear the audio ringbuffer when using AC3 re-encoder

When the audio buffer was reset, we could find ourselves in a situation where a full audio frame may not have fitted at the end of the ring buffer. This would have resulted in channels being shifted for a short period of time when we rollled over in the buffer. This situation could only happen when using the AC3 encoder as we change the size of an audio frame half-way through.

When the AC3 re-encoder is in use, we now fully clear the audio buffer and fill it with zero. That way at worse we hear silence instead of crap and not have to worry about threading issues.

Add some extra logging that will clearly show an error should this condition occurs again. Will help troubleshoot should any issues remain.

Additionally, from Mark Spieth, fully reset any audio processing unit that could have cached old audio.

Should fix #10396 Thanks Mark for actually spotting what went wrong

Branch: master Changeset: f8d2de08e982bd646d0a3e430ec8799f7494f8f8

[blazej.lewcio@…]

Congrats! Problem fixed. I tested the commited code in master branch. Thanks.