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

Add AGC example #632

Conversation

UnknownSuperficialNight
Copy link
Contributor

@UnknownSuperficialNight UnknownSuperficialNight commented Oct 6, 2024

Here is a quick AGC example

I don't know how to use set_enabled() outside the setup area, so I'm using the experimental atomicbool

close #629

@dvdsk
Copy link
Collaborator

dvdsk commented Oct 7, 2024

so I'm using the experimental atomicbool

I rather not have examples using experimental features, since those might go away at any time.

Its clear the current method for changing a source outside of setup is hard to find or use. We will work on that, maybe by changing the API maybe with extensive documentation if no better API can be found.

For now an example showing how to do it might be very useful. So lets try and do it without atomicbool?

What you will need is PeriodicAccess it is missing an example. The best we have now is looking at the source code of the Sink.

Now we get something along the lines of this:

   let agc_enabled = Arc::new(Atomicbool::new(true));
   

   // Apply automatic gain control to the source
    let agc_source = source.automatic_gain_control(1.0, 4.0, 0.005, 5.0);

   // Make the audio thread check every 5 millis if the agc needs 
   // to be enabled/disabled
   let controlled = agc_source.periodic_access(Duration::from_millis(5), 
     |periodic_access_source| {
        let agc_source = periodic_access_source.inner_mut()
        agc_source.set_enabled(agc_enabled.load(Relaxed));
     });

   // put it in sink & play
   // from some other part of the program:
   agc_enabled.store(true, Relaxed); // takes effect after 5 millis.

@UnknownSuperficialNight
Copy link
Contributor Author

UnknownSuperficialNight commented Oct 7, 2024

so I'm using the experimental atomicbool

I rather not have examples using experimental features, since those might go away at any time.

Its clear the current method for changing a source outside of setup is hard to find or use. We will work on that, maybe by changing the API maybe with extensive documentation if no better API can be found.

For now an example showing how to do it might be very useful. So lets try and do it without atomicbool?

What you will need is PeriodicAccess it is missing an example. The best we have now is looking at the source code of the Sink.

Now we get something along the lines of this:

   let agc_enabled = Arc::new(Atomicbool::new(true));
   

   // Apply automatic gain control to the source
    let agc_source = source.automatic_gain_control(1.0, 4.0, 0.005, 5.0);

   // Make the audio thread check every 5 millis if the agc needs 
   // to be enabled/disabled
   let controlled = agc_source.periodic_access(Duration::from_millis(5), 
     |periodic_access_source| {
        let agc_source = periodic_access_source.inner_mut()
        agc_source.set_enabled(agc_enabled.load(Relaxed));
     });

   // put it in sink & play
   // from some other part of the program:
   agc_enabled.store(true, Relaxed); // takes effect after 5 millis.

Yea, when I try periodic_access I can just never get it to work

Example:

error[E0599]: no method named `inner_mut` found for mutable reference `&mut AutomaticGainControl<Decoder<BufReader<File>>>` in the current scope
  --> examples/automatic_gain_control.rs:27:53
   |
27 |             let agc_source = periodic_access_source.inner_mut();
   |                                                     ^^^^^^^^^ method not found in `&mut AutomaticGainControl<Decoder<BufReader<File>>>`

The only thing I could get the compiler to agree with is this:

let controlled =
        agc_source.periodic_access(Duration::from_millis(5), move |src| {
            src.set_enabled(agc_enabled.load(Relaxed));
        });

But of course that's not very useful as you can't use it

Here is a code snippet for context:

    let agc_enabled = Arc::new(AtomicBool::new(true));

    // Apply automatic gain control to the source
    let agc_source = source.automatic_gain_control(1.0, 4.0, 0.005, 5.0);

    // Make the audio thread check every 5 millis if the agc needs
    // to be enabled/disabled
    let controlled =
        agc_source.periodic_access(Duration::from_millis(5), |periodic_access_source| {
            let agc_source = periodic_access_source.inner_mut();
            agc_source.set_enabled(agc_enabled.load(Relaxed));
        });

    // Add the controlled source to the sink for playback
    sink.append(controlled);

    // Keep the program running until playback is complete
    sink.sleep_until_end();

@dvdsk
Copy link
Collaborator

dvdsk commented Oct 7, 2024

ahh so the compile error states the type given to the closure is already the AutomaticGainControl. So that let agc_source bit that I give you is unneeded. Try something like |agc| agc.set_enabled(agc_enabled.load(Relaxed)).

It also points to a small issue with the agc source. It should have an inner mut and inner method. They are not part of Source since a source can be generating sound, thats why I forgot them while reviewing 😅.

@dvdsk dvdsk force-pushed the master branch 3 times, most recently from b10961c to a7f67b3 Compare October 10, 2024 00:14
@dvdsk
Copy link
Collaborator

dvdsk commented Oct 17, 2024

If this is still on your list let me know, there is no rush to finish this. However if you would rather leave it ill take it over. I would like to have this merged before releasing 0.20

@dvdsk dvdsk added this to the 0.20 milestone Oct 17, 2024
@UnknownSuperficialNight
Copy link
Contributor Author

If this is still on your list let me know, there is no rush to finish this. However if you would rather leave it ill take it over. I would like to have this merged before releasing 0.20

Sorry, I got a bit distracted with my own project and totally forgot about this. I did try to fiddle with the code a bit more to get the compiler to play nice, but no luck. If you want to take it over, go for it! I’d love to know what solution you find, so if you don't mind, let me know here when/if you get it working. For whatever reason, I can only get the atomicbool to agree with it. If it comes down to it, and we can’t get the compiler to cooperate, let’s just use the experimental one—it’s better than having no example at all.

@UnknownSuperficialNight UnknownSuperficialNight marked this pull request as ready for review October 17, 2024 17:29
@UnknownSuperficialNight
Copy link
Contributor Author

Marking it as ready for review so it can be edited and merged if you need to

@dvdsk
Copy link
Collaborator

dvdsk commented Oct 17, 2024

I’d love to know what solution you find, so if you don't mind, let me know here when/if you get it working.

Ill jump on it, im gonna merge this first and then directly edit on main as thats a little easier then editing the PR. Ill let you know once thats done.

@dvdsk dvdsk merged commit 71d9a6d into RustAudio:master Oct 17, 2024
11 checks passed
@dvdsk
Copy link
Collaborator

dvdsk commented Oct 17, 2024

see 48408bc for my approach

@UnknownSuperficialNight
Copy link
Contributor Author

UnknownSuperficialNight commented Oct 18, 2024

Ah, I see what I was not doing

I didn't add the move in

let controlled = agc_source.periodic_access(Duration::from_millis(5), move |agc_source| {
        agc_source.set_enabled(agc_enabled_clone.load(Ordering::Relaxed));
    });

and I didn't add the inner_mut and inner to AGC itself

Thanks for the help :)

Much easier with an example of usage

Edit1:
A bit hacky to do it that way for sure, need a better API.

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.

add automatic_gain example?
2 participants