Skip to content

[BUG] Distortion on the second pipeline connected to demux #2529

Closed
Closed
[BUG] Distortion on the second pipeline connected to demux#2529
Assignees
jajanuszslawblauciak
Labels
CMLApplies to Comet Lake platformP1Blocker bugs or important featuresSDWSoundWireSynchronized PlaybackApplies to synchronized playback over multiple backend linksbugSomething isn't working as expected

Description

@bardliao
bardliao
opened on Mar 11, 2020

Describe the bug
Distortion can be seen on the second amp output with #2526 applied

To Reproduce

  1. run aplay
  2. listen to the output from both amps

Reproduction Rate
100%

Expected behavior
Clear output from both amps

Impact
annoyance

Environment

  1. Branch name and commit hash of the 2 repositories: sof (firmware/topology) and linux (kernel driver).
  2. Name of the topology file
    • Topology: sof-cml-rt711-rt1308-rt715.tplg
  3. Name of the platform(s) on which the bug is observed.
    • Platform: CML laptop with 2 rt1308s

Screenshots or console output
sof-logger-distortion.txt

Activity

  1. bardliao
    added
    bugSomething isn't working as expected
    on Mar 11, 2020

  2. bardliao commented on Mar 11, 2020

    @bardliao
    bardliao
    on Mar 11, 2020
    CollaboratorAuthor

    @slawblauciak I know the sof-log log is not sufficient for the audio quality issue. But can you check if you can find anything wrong with the sof-log log? In the meantime, I will try to do loopback to capture the audio data for you.

  3. slawblauciak
    self-assigned this
    on Mar 11, 2020

  4. bardliao commented on Mar 12, 2020

    @bardliao
    bardliao
    on Mar 12, 2020
    CollaboratorAuthor

    @slawblauciak I did a loopback test today, and I can get the same distortion as what I hear. An interesting thing is that the sound is very clear when I test it with a 1K tone.
    Below are the recording file captured by loopback. The first picture is 1k tone and the second one is a voice says left channel and right channel repeatedly.
    1k

    lr

    I also upload the captured wav here

    1k.zip
    lr.zip

    Could you look into it?

  5. slawblauciak commented on Mar 12, 2020

    @slawblauciak
    slawblauciak
    on Mar 12, 2020
    Collaborator

    I'm on it. Can you also reproduce the issue by generating some other tone? Like 1.5K? Or 2K?

  6. bardliao commented on Mar 12, 2020

    @bardliao
    bardliao
    on Mar 12, 2020
    CollaboratorAuthor

    @slawblauciak That is the result of testing with your testing file.
    sine
    sine.zip

  7. bardliao commented on Mar 13, 2020

    @bardliao
    bardliao
    on Mar 13, 2020
    CollaboratorAuthor

    @slawblauciak The distortion is gone if I revert #2541. I also tried adding a PGA to the second DAI, and the distortion happens. So, what I observed is the distortion will happen if a PGA on the second pipeline. In other words, the second pipeline can only work with pass through pipeline.

  8. bardliao commented on Mar 13, 2020

    @bardliao
    bardliao
    on Mar 13, 2020
    CollaboratorAuthor

    I also don't observe distortion if I play a 32bits wav no matter if a PGA on the second pipeline or not. @RanderWang Do you have any comments?

  9. slawblauciak commented on Mar 13, 2020

    @slawblauciak
    slawblauciak
    on Mar 13, 2020
    Collaborator

    @bardliao looking at the #2541: I don't really understand why it was like that to begin with. Why apply volume to only one of the demux outputs. It is an interesting finding tho, it's very strange.

  10. bardliao commented on Mar 13, 2020

    @bardliao
    bardliao
    on Mar 13, 2020
    CollaboratorAuthor

    @bardliao looking at the #2541: I don't really understand why it was like that to begin with. Why apply volume to only one of the demux outputs. It is an interesting finding tho, it's very strange.

    @slawblauciak The reason of #2541 is that I want to keep the same volume level for two pipelines. @RanderWang said he think the issue probably due to bad format conversion. ie, convert 16/24 bits to 32 bits see https://github.com/thesofproject/sof/blob/master/src/audio/dai.c#L769.

  11. slawblauciak commented on Mar 13, 2020

    @slawblauciak
    slawblauciak
    on Mar 13, 2020
    Collaborator

    I think there's a problem with stream param propagation, also why you had to do that hack with the 2 channels. So, I think it might all be related. I'm currently investigating that.

  12. mengdonglin
    added
    CMLApplies to Comet Lake platform
    P1Blocker bugs or important features
    SDWSoundWire
    Synchronized PlaybackApplies to synchronized playback over multiple backend links
    on Mar 17, 2020

  13. 4 remaining items

  14. jajanusz commented on Mar 18, 2020

    @jajanusz
    jajanusz
    on Mar 18, 2020
    Contributor

    @bardliao Please add logs that show which method conversion was chosen in dai_params, so we'll know if demux is propagating correct formats.
    After this:

    sof/src/audio/dai.c

    Line 279 in b20191b

    dd->process = pcm_get_conversion_function(local_fmt, dd->frame_fmt);

  15. bardliao commented on Mar 18, 2020

    @bardliao
    bardliao
    on Mar 18, 2020
    CollaboratorAuthor

    I've seen that comment, but what you want me to do with it? It looks like HW requirement so it has to stay 32bit, if it doesn't work for non-32bit formats then volume has bad conversion logic (volume doesn't have conv, dai has) or topology doesn't make sense.

    @jajanusz I think the issue could be related to dconfig->frame_fmt = SOF_IPC_FRAME_S32_LE;. Could you check it first?

    I'd like to but we don't have repro, so it's hard to check. That's why I ask for all these things, so we can get repro on our side ;)

    @jajanusz Please below is the environment to repro it.
    Platform: Dell CML-H laptop
    kernel: https://github.com/bardliao/linux/commits/synchronize-playback
    FW&Tplg: https://github.com/bardliao/sof/commits/demux-test

  16. bardliao commented on Mar 18, 2020

    @bardliao
    bardliao
    on Mar 18, 2020
    CollaboratorAuthor

    @bardliao Please add logs that show which method conversion was chosen in dai_params, so we'll know if demux is propagating correct formats.
    After this:

    sof/src/audio/dai.c

    Line 279 in b20191b

    dd->process = pcm_get_conversion_function(local_fmt, dd->frame_fmt);

    dd->frame_fmt is always 2, and local_fmt depends on the testing wav format. ie. It is 0 for S16_LE and 2 for S32_LE. But I don't know how to check if we use functions defined in pcm_converter_generic.c or pcm_converter_hifi3.c.

  17. jajanusz commented on Mar 18, 2020

    @jajanusz
    jajanusz
    on Mar 18, 2020
    Contributor

    Can you change 0x02 in demux blob (it takes format from there instead from pipeline)?

    CONTROLBYTES_PRIV(DEMUX_priv,
`       bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
`       0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       >>0x02<< CHANGE THIS,0x00,0x02,0x00,0x02,0x00,0x00,0x00,'

    There should be 0x00 for 16 bits.
    So it'll look more like:

    CONTROLBYTES_PRIV(DEMUX_priv,
`       bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
`       0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x02,0x00,0x02,0x00,0x00,0x00,'

    Let me know if that helps.

  18. bardliao commented on Mar 19, 2020

    @bardliao
    bardliao
    on Mar 19, 2020
    CollaboratorAuthor

    Can you change 0x02 in demux blob (it takes format from there instead from pipeline)?

    CONTROLBYTES_PRIV(DEMUX_priv,
`       bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
`       0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       >>0x02<< CHANGE THIS,0x00,0x02,0x00,0x02,0x00,0x00,0x00,'

    There should be 0x00 for 16 bits.
    So it'll look more like:

    CONTROLBYTES_PRIV(DEMUX_priv,
`       bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
`       0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x02,0x00,0x02,0x00,0x00,0x00,'

    Let me know if that helps.

    @jajanusz With this change, S16_LE file can play but S32_LE is noisy.

  19. jajanusz commented on Mar 19, 2020

    @jajanusz
    jajanusz
    on Mar 19, 2020
    Contributor

    Can you change 0x02 in demux blob (it takes format from there instead from pipeline)?

    CONTROLBYTES_PRIV(DEMUX_priv,
`       bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
`       0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       >>0x02<< CHANGE THIS,0x00,0x02,0x00,0x02,0x00,0x00,0x00,'

    There should be 0x00 for 16 bits.
    So it'll look more like:

    CONTROLBYTES_PRIV(DEMUX_priv,
`       bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
`       0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x02,0x00,0x02,0x00,0x00,0x00,'

    Let me know if that helps.

    @jajanusz With this change, S16_LE file can play but S32_LE is noisy.

    That's expected behaviour, you have to match format there with your format.

  20. emilchudzik commented on Mar 19, 2020

    @emilchudzik
    emilchudzik
    on Mar 19, 2020
    Contributor

    @bardliao I have CML-H as you:
    Platform: Dell CML-H laptop

    I use your kernel and FW and topology.
    kernel: https://github.com/bardliao/linux/commits/synchronize-playback
    FW&Tplg: https://github.com/bardliao/sof/commits/demux-test

    But I have no repro with it.

    Then I try with your patch: #2533 and still no repro.

    I use alsamixer to volume up controls.
    Then I use command to play:
    sudo aplay -vv -Dhw:1,2 -f dat - c2 doremi-48k-s16le-stereo.wav
    or
    sudo aplay -vv -Dhw:1,2 doremi-48k-s16le-stereo.wav

    I have always clear sound on both speakers, without noise.

    Did you modify topology? I got this one: sof-cml-rt711-rt1308-rt715.tplg from your branch

  21. emilchudzik commented on Mar 19, 2020

    @emilchudzik
    emilchudzik
    on Mar 19, 2020
    Contributor

    Could you share how you made loopback? I see in comments that you record to file output, so you have to modify toplogy to made loopback, right?

  22. bardliao commented on Mar 19, 2020

    @bardliao
    bardliao
    on Mar 19, 2020
    CollaboratorAuthor

    Can you change 0x02 in demux blob (it takes format from there instead from pipeline)?

    CONTROLBYTES_PRIV(DEMUX_priv,
`       bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
`       0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       >>0x02<< CHANGE THIS,0x00,0x02,0x00,0x02,0x00,0x00,0x00,'

    There should be 0x00 for 16 bits.
    So it'll look more like:

    CONTROLBYTES_PRIV(DEMUX_priv,
`       bytes "0x53,0x4f,0x46,0x00,0x00,0x00,0x00,0x00,'
`       0x28,0x00,0x00,0x00,0x00,0x60,0x00,0x03,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,'
`       0x00,0x00,0x02,0x00,0x02,0x00,0x00,0x00,'

    Let me know if that helps.

    @jajanusz With this change, S16_LE file can play but S32_LE is noisy.

    That's expected behaviour, you have to match format there with your format.

    But, the problem is we don't know what format will user play. Besides, there is no issue on the 1st pipeline connected to demux. The issue happens only on the 2nd pipeline.

  23. bardliao commented on Mar 19, 2020

    @bardliao
    bardliao
    on Mar 19, 2020
    CollaboratorAuthor

    @bardliao I have CML-H as you:
    Platform: Dell CML-H laptop

    I use your kernel and FW and topology.
    kernel: https://github.com/bardliao/linux/commits/synchronize-playback
    FW&Tplg: https://github.com/bardliao/sof/commits/demux-test

    But I have no repro with it.

    Then I try with your patch: #2533 and still no repro.

    Sorry, it should be #2541. I applied it to my repo, too
    https://github.com/bardliao/sof/commits/demux-test

    I use alsamixer to volume up controls.
    Then I use command to play:
    sudo aplay -vv -Dhw:1,2 -f dat - c2 doremi-48k-s16le-stereo.wav
    or
    sudo aplay -vv -Dhw:1,2 doremi-48k-s16le-stereo.wav

    I have always clear sound on both speakers, without noise.

    Did you modify topology? I got this one: sof-cml-rt711-rt1308-rt715.tplg from your branch

    No, I didn't modify topology manually and yes, sof-cml-rt711-rt1308-rt715.tplg is the right one.

  24. bardliao commented on Mar 19, 2020

    @bardliao
    bardliao
    on Mar 19, 2020
    CollaboratorAuthor

    Could you share how you made loopback? I see in comments that you record to file output, so you have to modify toplogy to made loopback, right?

    Ok, I will do it.

  25. emilchudzik commented on Mar 19, 2020

    @emilchudzik
    emilchudzik
    on Mar 19, 2020
    Contributor

    @bardliao I have CML-H as you:
    Platform: Dell CML-H laptop
    I use your kernel and FW and topology.
    kernel: https://github.com/bardliao/linux/commits/synchronize-playback
    FW&Tplg: https://github.com/bardliao/sof/commits/demux-test
    But I have no repro with it.
    Then I try with your patch: #2533 and still no repro.

    Sorry, it should be #2541. I applied it to my repo, too
    https://github.com/bardliao/sof/commits/demux-test

    My mistake, I used #2541, not 2533 (error when pasting here)

    I use alsamixer to volume up controls.
    Then I use command to play:
    sudo aplay -vv -Dhw:1,2 -f dat - c2 doremi-48k-s16le-stereo.wav
    or
    sudo aplay -vv -Dhw:1,2 doremi-48k-s16le-stereo.wav
    I have always clear sound on both speakers, without noise.
    Did you modify topology? I got this one: sof-cml-rt711-rt1308-rt715.tplg from your branch

    No, I didn't modify topology manually and yes, sof-cml-rt711-rt1308-rt715.tplg is the right one.

  26. bardliao commented on Mar 19, 2020

    @bardliao
    bardliao
    on Mar 19, 2020
    CollaboratorAuthor

    @emilchudzik The loopback environment is:
    kernel: https://github.com/bardliao/linux/commits/demux-loopback-test
    sof: https://github.com/bardliao/sof/commits/demux-looback-test
    Please run below script before testing.
    rt711_mixer.txt
    rt1308_mixer.txt
    Open two terminal.
    run arecord and aplay at the same time (start arecord before aplay)
    arecord -Dhw:0,1 -fDAT loop.wav -vv
    aplay -Dhw:0,2 test.wav -vv

    Check loop.wav.
    We do loopback from headset port. You can hear the noise from headset and only left speaker will output. And aplay -Dhw:0,0 will not work.

  27. tlauda commented on Mar 20, 2020

    @tlauda
    tlauda
    on Mar 20, 2020
    Contributor

    Fixed by #2589.

  28. tlauda
    closed this as completedon Mar 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

CMLApplies to Comet Lake platformP1Blocker bugs or important featuresSDWSoundWireSynchronized PlaybackApplies to synchronized playback over multiple backend linksbugSomething isn't working as expected

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions