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

freewheeling (was OSC doc) #10

Merged
merged 2 commits into from May 26, 2012
Merged

freewheeling (was OSC doc) #10

merged 2 commits into from May 26, 2012

Conversation

x42
Copy link
Contributor

@x42 x42 commented May 24, 2012

Re-using this "already open pull request"

OSC documentation: done
please proof-read and see it it makes sense :)

@x42
Copy link
Contributor Author

x42 commented May 24, 2012

PS. maybe this can all go into --advanced-options

That help is already loong enough a few more lines won't hurt. OTOH, personally I prefer if usage information is structured rather than a big blob: I don't want to read about OSC unless I want to know about OSC. :) YMMV.

@x42
Copy link
Contributor Author

x42 commented May 24, 2012

can you fix the typo on merge? I guess that's faster than opening another pull request...
I'm OK with including it regardless of HAVE_LIBLO..
This is your baby :)

@x42
Copy link
Contributor Author

x42 commented May 24, 2012

I don't get any over or underruns (because the buffer increases a LOT) - but you're right about the autoincrease callback. Once that is in place, there are lots of internal x-runs.

This needs a bit more work..

@x42
Copy link
Contributor Author

x42 commented May 24, 2012

in freewheeling- mode: actually the jack_callback should wait until the buffer is flushed!

@kmatheussen
Copy link
Owner

Sorry for late reply, I didn't see your comments here. Just wait until flushed is probably the simplest.

Is your tree working properly now? Should I merge it into my branch?

@x42
Copy link
Contributor Author

x42 commented May 26, 2012

Sorry, I did mess up this pull request. Lemme see if I can clean it up a bit, since you've already merged the OSC doc.
While most github features are very cool, there's still some rough edges -- or maybe that's just me :)

yes, the it is working properly -- but I have only tested
jack_capture -jf
jack_freewheel y
jack_freewheel n

and not any edge cases that may or may not exist.

@x42
Copy link
Contributor Author

x42 commented May 26, 2012

Right, it took a push --force, because I've rebased the two commits, but it's clean now.

@@ -2112,6 +2139,10 @@ void init_arguments(int argc, char *argv[]){
OPTARG_LAST() base_filename=OPTARG_GETSTRING();
}OPTARGS_END;

if(use_jack_freewheel==true && use_jack_transport==true){
fprintf(stderr,"--jack-transport and --jack-freewheel are exclusive options.\n");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe this should read "mutually exclusive" ?!

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, that sounds more correct.

@x42
Copy link
Contributor Author

x42 commented May 26, 2012

BTW it also misses a ChangeLog entry.. But please review the code first and I'll do all corrections after that.
Cheers!
robin

@kmatheussen
Copy link
Owner

Hmm, waiting a couple of milliseconds could cause freewheeling to take very long time to finish.

I think that instead of waiting until it's flushed, just wait until enough space is available before writing.
That should exploit use of multiple processors as well.

@kmatheussen
Copy link
Owner

I think this is much better:

static bool process_new_current_buffer(int frames_left){
while (vringbuffer_get_writing_size(vringbuffer)==0)
usleep(2*1000);
...
}

@x42
Copy link
Contributor Author

x42 commented May 26, 2012

Ow, too late :)

Well why don't you go ahead, merge it and fix it. That's much faster, you know your way around much better than I do and it's a few lines only..

@kmatheussen
Copy link
Owner

No problem. But now I wonder if it's a good idea... If we're using the buffer to its fullest, freewheeling could finish a long time before the sound is actually written to the disk. Do you know if that could be a problem?

@kmatheussen
Copy link
Owner

jack_capture will of course write the complete sound to disk, it won't just cut off. But it could take some time before it finishes. Perhaps a script or something could expect the file to be available directly after freewheeling is finished.

kmatheussen added a commit that referenced this pull request May 26, 2012
freewheeling (was OSC doc)
@kmatheussen kmatheussen merged commit bc51f77 into kmatheussen:master May 26, 2012
@x42
Copy link
Contributor Author

x42 commented May 26, 2012

good points. Compared it to Fons' tool: it keeps sleeping for 1/2 seconds in main until freewheeling mode ends and then closes the file. So when jack issues a "jack-freewheeling ended" callback, the file may not yet be complete on disk.

I can't think of a good use-case that would require the file to be flushed to disk when jack issues the callback but that does not mean there is none. -- Worst case would have to wait until jack_capture exits.

re buffer-usage: right, it would be a bit faster, but I think this is a non-issue.
If we can assure that the file is complete when jack-freewheeling ends, it'll be a feature that Fons' tool hasn't :)

@x42
Copy link
Contributor Author

x42 commented May 26, 2012

actually the usleep() is ugly. -- it should block on a signal from the disk-writer thread rather than wait and poll it.

on that note: can it happen that vringbuffer_get_reading_size(vringbuffer) won't return to 0?

the receiver_func() has a
while(vringbuffer_reading_size(vrb) > 0) /* flush stuff to disk */
so it seems OK.. but there may be edge-cases that I'm not aware of.

@kmatheussen
Copy link
Owner

I think we should keep usleep() calls, (BTW, there is an msleep function available in jack_capture.c), since sleeping is simpler to understand than signalling.

Also, I just tried to wait until the buffer is availble, instead of waiting for the buffer to flush, and it seems 10 times faster now!

To ensure the buffer is fully written, we can just flush it in the freewheel callback, when the argument is 0.
Adding that code now.

@kmatheussen
Copy link
Owner

I'm pretty sure that vringbuffer_get_writing_size(vringbuffer) must return 0 after a while. If not, we would have noticed glitches in sound. vringbuffer would be buggy if it sometimes wouldn't return 0 after a while.

@kmatheussen
Copy link
Owner

I meant vringbuffer_reading_size

@kmatheussen
Copy link
Owner

Fixed here:
18419bf

@kmatheussen
Copy link
Owner

I think you're right. vringbuffer_reading_size can actually not return 0 after a while. We need to call sem_post() to ensure. Must fix.

@kmatheussen
Copy link
Owner

That's a really good catch by the way!

@kmatheussen
Copy link
Owner

Think this is enough:
064519b

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.

None yet

2 participants