thesofproject / sof

Sound Open Firmware
Other
530 stars 307 forks source link

[BUG]:EQ FIR and IIR use the same set of random numbers sequence. #7662

Open ShriramShastry opened 1 year ago

ShriramShastry commented 1 year ago

Describe the bug A clear and concise description of what the bug is. What have you tried to diagnose or workaround this issue? Please also read https://thesofproject.github.io/latest/contribute/process/bug-tracking.html for further information on submitting bugs.

The random number's goal is to always produce a new series of numbers while stressing the function, yet rand() produces the same sequence for testing rather than a separate set of numbers. 

This is because default srand() is 1, since the start of sequence didn't change we are testing with same  number.
`test/cmocka/src/audio/eq_fir/eq_fir_process.c` and `\test\cmocka\src\audio\eq_iir\eq_iir_process.c`

To Reproduce

Steps to reproduce the behavior: (e.g. list commands or actions used to reproduce the bug)
Introduce srand(x), where x does not equal one.

Reproduction Rate How often does the issue happen ? i.e. 1/10 (once in ten attempts), 1/1000 or all the time. Does the reproduction rate vary with any other configuration or user action, if so please describe and show the new reproduction rate.

It is impossible to comprehend the failure rate given that the test consistently employs the same sequence and, in the current context, always yields positive results. The FIR function ought to be assessed using a random number sequence that is changing.

Expected behavior A clear and concise description of what you expected to happen. When evaluating the eq_fir function, a variety of random number aids can be used to identify FIR faults.

Impact What impact does this issue have on your progress (e.g., annoyance, showstopper) Because the test is evaluated with a repeated sequence, it always works, but we don't know how FIR acts with a constantly changing collection of data, and with a known sequence, there's no way to detect weaknesses in the system.

Environment 1) Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).

Screenshots or console output If applicable, add a screenshot (drag-and-drop an image), or console logs (cut-and-paste text and put a code fence (```) before and after, to help explain the issue.

static int frames_jitter(int frames)
{
    int r = rand(); // this should be **srand(x)**

    if (r > THR_RAND_PLUS_ONE)
        return frames + 1;
    else if (r < THR_RAND_MINUS_ONE)
        return frames - 1;
    else
        return frames;
}

Please also include the relevant sections from the firmware log and kernel log in the report (and attach the full logs for complete reference). Kernel log is taken from dmesg and firmware log from sof-logger. See https://thesofproject.github.io/latest/developer_guides/debugability/logger/index.html

plbossart commented 1 year ago

It's not clear to me that you really want different random values every time you test. There's also a need to reproduce the test results....

ShriramShastry commented 1 year ago

The objective of introducing the random number is to test different sets of numbers, as opposed to the present random numbers, which are repetitious.

The current code function behavior cannot be understood because it is not accessible with new data, and the accuracy parameter is not guaranteed with new data.

Essentially, enhancement is required so that we can include fresh data into the mockup design.

lgirdwood commented 1 year ago

@singalsu @ShriramShastry can we add the randomness to the UTs (i.e. a new class of random UT which must dump full test data for repoducibility).

ShriramShastry commented 1 year ago

I intend to start working. Yes, I will take your recommendation of a full test data dump into account.