ebur128_init_resampler unexpected oversample rates

[evan314159]

In ebur128_init_resampler, oversampling rates are changed at <96000 and <192000. This creates unexpected oversample rates with standard audio rates from 44.1kHz, and expected oversampling up to 176.4/192kHz.

input rate  libebur128 oversample rate  expected oversample rate
44100       176400 (4x)                 176400 (4x)
48000       192000 (4x)                 192000 (4x)
88200       352800 (4x)                 176400 (2x)
96000       192000 (2x)                 192000 (2x)
176400      352800 (2x)                 no oversample
192000      no oversample               no oversample

The oversample rates should instead change at <88200 and <176400. Original code:

static int ebur128_init_resampler(ebur128_state* st) {
  int errcode = EBUR128_SUCCESS;

  if (st->samplerate < 96000) { // should be 88200
    st->d->interp = interp_create(49, 4, st->channels);
    CHECK_ERROR(!st->d->interp, EBUR128_ERROR_NOMEM, exit)
  } else if (st->samplerate < 192000) { // should be 176400
    st->d->interp = interp_create(49, 2, st->channels);
    CHECK_ERROR(!st->d->interp, EBUR128_ERROR_NOMEM, exit)
  } else {

[bmatherly]

My understanding of the standard is that the point of oversampling is to end up with a sample rate that is "at least" 192k. Your suggestion results in a Fs of 176400 which is less than 192000. So I don't think is what the standard has in mind.

However, your table exposes a problem that for Fs < 48000, the oversampled frequency is still less than 192k. So maybe we should add a test for that:

  if (st->samplerate <48000) {
    st->d->interp = interp_create(49, 8, st->channels);
    CHECK_ERROR(!st->d->interp, EBUR128_ERROR_NOMEM, exit)
  } else  if (st->samplerate < 96000) {
    st->d->interp = interp_create(49, 4, st->channels);
    CHECK_ERROR(!st->d->interp, EBUR128_ERROR_NOMEM, exit)
  } else if (st->samplerate < 192000) {
    st->d->interp = interp_create(49, 2, st->channels);
    CHECK_ERROR(!st->d->interp, EBUR128_ERROR_NOMEM, exit)
  } else {

I went back to check BS.1770 (which is referenced by EBU r128) to see what it says. As a broadcast standard, it seems to assume that Fs will be 48000 and isn't crystal clear what to do about other sampling frequencies.

Let me know what you think:
https://www.itu.int/dms_pubrec/itu-r/rec/bs/R-REC-BS.1770-5-202311-I!!PDF-E.pdf

[evan314159]

You are right as far as the literal text, but I don't think that was the intent -- as you say video uses multiples of 48k only, so 44.1 was probably not a consideration.

Whichever position one takes, the current behaviour is not correct.

[bmatherly]

If someone wants to apply and test my proposed patch, I am open to that. But we should not reduce the oversampling factor for 176400 since that would risk a regression.

[evan314159]

How would one test this, in the sense that the oversampling rate is not really exposed to the outside? Add debug printfs?

The proposed change looks correct as per the way the specification is written as long as the source is 24kHz or higher, and importantly covers 44.1kHz.

[jiixyj]

Looks good to me as well! We could even expand the table for lower sample rates, just as for example MATLAB does (https://de.mathworks.com/help/audio/ref/loudnessmeter-system-object.html):