Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crackling Unclean Audio in 1080i5994 #387

Closed
ronag opened this issue Oct 26, 2015 · 20 comments
Closed

Crackling Unclean Audio in 1080i5994 #387

ronag opened this issue Oct 26, 2015 · 20 comments

Comments

@ronag
Copy link
Member

ronag commented Oct 26, 2015

Running a channel in 1080i5994 results in crackling audio.

2.0.7

@HellGore
Copy link
Contributor

I tried with system-audio and with a Decklink 4K Extreme 6G but I can't reproduce this. Could you provide more information about your system setup and perhaps a test-file you have used when you observed this problem?

@ronag
Copy link
Member Author

ronag commented Oct 27, 2015

Decklink Studio, 1080i5994

The distortion is small you need to listen quite carefully for it.

@HellGore
Copy link
Contributor

I have downloaded the file now, so you can remove it if keeping the file public is a problem

@HellGore
Copy link
Contributor

Its actually interesting, I get the same audio pops when running the file on a 1080i5000 channel, the audio is the same (video of course out of sync). When looking at the audio meter in the client (fed via OSC) I can see that the pops seems to be when the volume goes up to 0dbf, so it seems to be an audio amplitude issue rather than an audio cadence issue. I'm guessing something with the integer -> float -> integer conversions in the audio_mixer. Probably what we are seeing is an overflow in the calculation so that a sample becomes negative instead of positive or vice versa.

@HellGore
Copy link
Contributor

I can verify if I do a MIXER 1-10 VOLUME 0.9 I don't get the pops, so it is something with the audio mixer amplitude

@ronag
Copy link
Member Author

ronag commented Oct 27, 2015

Yes

This looks a little suspicious (row 293):

return caspar::array<int32_t>(result.data(), result.size(), true, std::move(result_owner));

@HellGore
Copy link
Contributor

Yes I changed it so that caspar::array is used for audio in 2.1.0 (already used for image data) to be able to have different type erased data owners (it is actually an av_frame in cases when the pan-filter has been used) to avoid unnecessary copying.

However this is not in 2.0.7 so the problem is probably something else in audio_mixer.cpp (same in 2.0 and 2.1, both gets the audio pop)

@ronag
Copy link
Member Author

ronag commented Oct 27, 2015

Aah, sorry, I still get confused about which branch is the default one.

@ronag
Copy link
Member Author

ronag commented Oct 27, 2015

Yea, I have no idea what is going on with the audio code. Is a bit over my head. Think I'll go with the VOLUME workaround for now.

Where we go from float to int (not sure where that is) we should probably use http://www.boost.org/doc/libs/1_42_0/libs/numeric/conversion/doc/html/index.html.

@deedos
Copy link

deedos commented Oct 27, 2015

This might be related #326

2015-10-27 11:25 GMT-02:00 Robert Nagy notifications@github.com:

Yea, I have no idea what is going on with the audio code. Is a bit over my
head. Think I'll go with the VOLUME workaround for now.


Reply to this email directly or view it on GitHub
#387 (comment).

Daniel Roviriego
(21) 972361654

@HellGore
Copy link
Contributor

The problem is that std::numeric_limits<int32_t>::max() is 2147483647 but static_cast<float>(2147483647) results in 2147483648.0f (+1 from the original) which when casted back to int32_t overflows to -2147483648.

So we have to think of a way of "clipping" on the right side of the 0 line instead of overflowing via static_cast

@HellGore
Copy link
Contributor

First static_casting to int64_t and then truncating via std::min() and std::max() and then static_casting back to int32_t would work but I don't know about the performance hit when doing this for each sample.

@ronag
Copy link
Member Author

ronag commented Oct 27, 2015

The performance hit should be negligible. Audio isn't all that much data.

@HellGore
Copy link
Contributor

OK, good.

Btw, do you remember if there is there a reason why we are mixing with float instead of using int32_t all the way inside the audio_mixer?

@ronag
Copy link
Member Author

ronag commented Oct 27, 2015

Hm, no I don't remember. Should be fine to use int32_t.

@ronag
Copy link
Member Author

ronag commented Oct 27, 2015

Ok. From my test this issue does not seem to be related to the audio levels. I just halved the level and it's the same problem.

@ronag
Copy link
Member Author

ronag commented Oct 27, 2015

It's a cadence issue. The audio mixer warns for appended zero samples.

@ronag
Copy link
Member Author

ronag commented Oct 27, 2015

It's a bug in the audio mixer with interlacing + NTSC audio cadence. In audio mixer add:

BOOST_FOREACH(auto& item, items_)
{
    if (next_audio_streams.find(item.tag) != next_audio_streams.end())
       continue;
  // ...

@HellGore
Copy link
Contributor

I think you have fixed the cadence issue in 2.1 actually.

Btw, I guess the only expected case of a cadence error occurring should be in the beginning of an audio_stream right? Perhaps we should be prepending the silence instead of appending it. What do you think?

@ronag
Copy link
Member Author

ronag commented Oct 30, 2015

There is no cadence error if the code is correct.

The bug currently is that the first field will use the old samples (that are over from the previous frame) and append the frame's samples. The second field will simply throw away the old samples and write only the frame's samples. Therefore you get constant 2 sample underflows.

Note that both fields have the same samples but the second one will truncate instead of append to the sample stream. Thus if we simply ignore the second field the bug is fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants