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

Make mono to stereo conversion automatic (fixes fmradio) #455

Merged
merged 5 commits into from
Mar 17, 2023

Conversation

linknum23
Copy link
Contributor

Closes #454.

To test a nearby radio station is 91.7, verify that stereo audio is output on each source.

@linknum23 linknum23 requested a review from Lohrer March 14, 2023 21:29
@Lohrer
Copy link
Collaborator

Lohrer commented Mar 15, 2023

If you'd rather just use the auto mono->stereo conversion that the plug plugin uses, we can add a route plugin in between plug and dmix. The current problem is plug thinks it's output is 8 channels, not 2, and I think that is messing with the auto conversions. So the current situation: [1/2]->plug->[8]->dmix. [N]=>N channel audio.

With the following setup we'll get: [1/2]->plug->[2]->route->[8]->dmix

pcm.ch1 {
    type                plug
    slave.pcm {
        type            route
        slave.pcm       "dmixer"
        slave.channels  8
        ttable.0.6      0.64
        ttable.1.7      0.64
    }
    slave.channels      2
}
pcm.ch2 {
    type                plug
    slave.pcm {
        type            route
        slave.pcm       "dmixer"
        slave.channels  8
        ttable.0.0      0.64
        ttable.1.1      0.64
    }
    slave.channels      2
}
pcm.ch3 {
    type                plug
    slave.pcm {
        type            route
        slave.pcm       "dmixer"
        slave.channels  8
        ttable.0.4      0.64
        ttable.1.5      0.64
    }
    slave.channels      2
}

@linknum23 linknum23 changed the title Add mono to stereo conversion for fmradio Make mono to stereo conversion automatic fixing fmradio Mar 15, 2023
@linknum23 linknum23 changed the title Make mono to stereo conversion automatic fixing fmradio Make mono to stereo conversion automatic (fixes fmradio) Mar 15, 2023
@linknum23
Copy link
Contributor Author

Oddly this does not work with vlc on ch1 and ch3. Works fine for ch2.

@linknum23
Copy link
Contributor Author

With the extra layer in play here is what the verbose output -vvv looks like from vlc:

[0204b6d8] alsa audio output debug: using ALSA device: ch1
[0204b6d8] alsa audio output debug:  Plug PCM: Route conversion PCM
  Transformation table:
    0 <- none
    1 <- none
    2 <- none
    3 <- none
    4 <- none
    5 <- none
    6 <- 0*0.64
    7 <- 1*0.64

Without the extra layer:

[0135d6d8] alsa audio output debug: using ALSA device: ch1
[0135d6d8] alsa audio output debug:  Plug PCM: Direct Stream Mixing PCM

@Lohrer
Copy link
Collaborator

Lohrer commented Mar 15, 2023

Not so odd I guess, ch2 uses physical channels 0 and 1 on the 8-channel CM6206, so no remapping is required. What is weird is that the FM radio stream (mono) and Spotify (stereo) both worked so I assumed the rest of the stereo streams would work...

My AmpliPi has been borrowed so I can't test, but I assume when you try ch3 you get:

Plug PCM: Route conversion PCM
  Transformation table:
    0 <- none
    1 <- none
    2 <- none
    3 <- none
    4 <- 0*0.64
    5 <- 1*0.64
    6 <- none
    7 <- none

@Lohrer
Copy link
Collaborator

Lohrer commented Mar 15, 2023

Those messages come from https://github.com/alsa-project/alsa-lib/blob/master/src/pcm/pcm_dmix.c and https://github.com/alsa-project/alsa-lib/blob/master/src/pcm/pcm_route.c and look exactly like what I'd expect. I think if there's an error it might lie elsewhere...

@linknum23
Copy link
Contributor Author

Another way to do this would be with 2 layers of plug:

pcm.ch1 {
    type                plug
    slave.pcm {
        type            plug
        slave.pcm       "dmixer"
        slave.channels  8
        ttable.0.6      0.64
        ttable.1.7      0.64
    }
    slave.channels      2
}
pcm.ch2 {
    type                plug
    slave.pcm {
        type            plug
        slave.pcm       "dmixer"
        slave.channels  8
        ttable.0.0      0.64
        ttable.1.1      0.64
    }
    slave.channels      2
}
pcm.ch3 {
    type                plug
    slave.pcm {
        type            plug
        slave.pcm       "dmixer"
        slave.channels  8
        ttable.0.4      0.64
        ttable.1.5      0.64
    }
    slave.channels      2
}

@linknum23
Copy link
Contributor Author

2 layers of plug works and was tested with Pandora, Airplay, Spotify, Internet Radio, and FM Radio

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2023

Codecov Report

Merging #455 (d1c89d9) into develop (17390bb) will increase coverage by 8.09%.
The diff coverage is 67.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@             Coverage Diff             @@
##           develop     #455      +/-   ##
===========================================
+ Coverage    56.25%   64.34%   +8.09%     
===========================================
  Files           12       15       +3     
  Lines         2839     4171    +1332     
===========================================
+ Hits          1597     2684    +1087     
- Misses        1242     1487     +245     
Flag Coverage Δ
unittests 64.34% <67.00%> (+8.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
amplipi/asgi.py 0.00% <0.00%> (ø)
amplipi/updater/asgi.py 0.00% <0.00%> (ø)
amplipi/rt.py 26.30% <23.07%> (-0.13%) ⬇️
amplipi/mpris.py 37.62% <37.62%> (ø)
amplipi/streams.py 46.11% <50.32%> (+4.48%) ⬆️
amplipi/utils.py 70.98% <71.87%> (-0.24%) ⬇️
amplipi/ctrl.py 80.99% <74.86%> (-1.44%) ⬇️
amplipi/app.py 76.97% <75.00%> (-8.42%) ⬇️
tests/test_rest.py 94.07% <97.87%> (ø)
amplipi/models.py 95.47% <100.00%> (+0.47%) ⬆️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor Author

@linknum23 linknum23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this setup makes the most sense going forward since it handles any weird cases where mono audio will get played. We will need to continue testing this to make sure there aren't any performance penalties with the double plug setup.

This allows us to use ALSA automatic mono -> stereo conversions.
This works and was tested with Pandora, Airplay, Spotify, Internet Radio, and FM Radio.
@linknum23
Copy link
Contributor Author

@Lohrer Do you think this makes sense?
Basic testing with 4 various streams playing seems to use about 15-20% for each stream which seems pretty reasonable.

@Lohrer Lohrer changed the base branch from main to develop March 16, 2023 14:42
Copy link
Collaborator

@Lohrer Lohrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I just think there's a some comments that might need to reflect the latest changes.

streams/fmradio.py Outdated Show resolved Hide resolved
@linknum23 linknum23 requested a review from Lohrer March 16, 2023 19:14
@linknum23 linknum23 merged commit 0e795cb into develop Mar 17, 2023
@linknum23 linknum23 deleted the mono-radio branch May 3, 2023 19:35
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

Successfully merging this pull request may close these issues.

RTL-SDR source playing only from left channel
3 participants