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

adding xruns_get command #1

Closed
wants to merge 1 commit into from
Closed

adding xruns_get command #1

wants to merge 1 commit into from

Conversation

kasbah
Copy link
Contributor

@kasbah kasbah commented Aug 1, 2013

So you likely don't want to merge this as is but I wanted to draw your attention to it anyway. This gives you an xruns_get command which responds:

resp 0 <number of xruns> <max delay usecs>

It resets the values when invoked. I am planning on using mod-host to evaluate the performance of a number of SoCs + Codec hardware combinations which is what I need this command for. I am not sure how useful it is to you guys.

I pulled in jack2 as a submodule so I can use JackAtomic.h though so if you do want to merge and are not happy with that then I am open to suggestions. Well, I am open to any comments or suggestions anyway actually.

@odurc
Copy link
Contributor

odurc commented Aug 7, 2013

Hey Kaspar!

First, thanks by pull request, it's really useful for us. But, we don't would like to make the mod-host code dependent of jack2 code. The use of JackAtomic it is really necessary?

@kasbah
Copy link
Contributor Author

kasbah commented Aug 7, 2013

Glad to hear that this is useful for you too.

There might be a good case for actually implementing this within jack and exposing these functions via the jack API. I will ask on the jack mailing list.

JackAtomic from jack2 is a nice, cross-architecture header for atomic operations. I could also just copy it as it hardly depends on the rest of jack2. It just seemed cleaner to defer the responsibility of maintaining and updating it to the jack2 developers.

Do you think we will need atomic operations for other things in mod-host though?

@bgola
Copy link
Contributor

bgola commented Aug 7, 2013

Hi :)

i think it would fit better to us if it used the monitor system. i mean, you would register a monitor for xruns and the host send a notification whenever there are N xruns or fixed timeout, i dont know...

but i don't see any problems adding xruns_get as well, the only thing is the submodule. we use a specific jack version on MOD hardware so it would be much better calling those functions using the jack API as you suggested.

anyway, do you think it is really necessary to use the atomic stuff in this case? if yes, can you explain?

I don't see any other stuff where we would need atomic operations.

thank you :)
gola

@kasbah
Copy link
Contributor Author

kasbah commented Aug 7, 2013

Sure, register a monitor, yeah that might be nicer and less overhead when not needed.

So the problem I have is that the XRun callback is being called from Jack. It can happen anytime; mod-host has no control. From mod-host we are reading the number that the XRun callbacks writes. If we do not make the write atomic or put a lock on it then we might read it when it is half-written giving us an invalid value. Vice versa we are writing the value to 0 outside of Jack which could get interrupted by the XRun callback.

So we either use locks or use atomic operations and I think atomic operations are nicer. But you are right If you can't envision using the atomic operations for anything else then it is a bit much to pull in so much extra code just for this.

I can see that you use the jack_ringbuffer in other places which I believe is a lockless ringbuffer which makes use of atomic operations internally. We could use that instead, though it's a little awkward given we only want to increment one value and don't need a ringbuffer for that.

@kasbah
Copy link
Contributor Author

kasbah commented Aug 31, 2013

I will re-write this using JackAtomic.h in the source.

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.

3 participants