capture_client modernization, a call for review

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

capture_client modernization, a call for review

Dmitry Baikov
Hi,

I made a more modern and robust version of examples/capture_client.c.
Now it uses semaphores and writes silence in place of overruns to keep timing.
This is the first version intended for initial review.

The example became a bit more complex, because ringbuffer lacks needed
functionality, but few changes to it (ringbuffer) (backward
compatible, of course) will solve the problem.

So, here's the patch for capture_client.

In case of positive feedback I'll refine it, prepare patches for
ringbuffer and will make playback_client in the same style (with
precise timing).

Best regards,

Dmitry.


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel

capture_client.diff (8K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Paul Davis
On Sat, 2006-07-15 at 03:42 +0400, Dmitry Baikov wrote:
> Hi,
>
> I made a more modern and robust version of examples/capture_client.c.
> Now it uses semaphores and writes silence in place of overruns to keep timing.
> This is the first version intended for initial review.

i appreciate the effort, but at this point you will have to explain why
its better than kjetil's new jack_capture.

> The example became a bit more complex, because ringbuffer lacks needed
> functionality, but few changes to it (ringbuffer) (backward
> compatible, of course) will solve the problem.

what functionality?




-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Dmitry Baikov
On 7/15/06, Paul Davis <[hidden email]> wrote:
> i appreciate the effort, but at this point you will have to explain why
> its better than kjetil's new jack_capture.
Opps, I missed it completely.
Took a look, and may say jack_capture is twice as large and still does
not compare: it uses cond/mutex and does not keep timing right. I
offer my patch for it also :)

Anyway, capture_client resides in examples dir for a purpose: it's a
_simple_ enough and short example how to write _good_ jack clients. It
touches very important topics: communication of process callback and
worker thread (simple message queue, which is nonblocking for
rt-thread, but blocking for worker-thread) and fault recovery (which
is often overlooked by app authors).

> > The example became a bit more complex, because ringbuffer lacks needed
> > functionality, but few changes to it (ringbuffer) (backward
> > compatible, of course) will solve the problem.
>
> what functionality?

read and write without commiting new pointer to other side. Something
like send(... MSG_MORE) in sockets.

I understand your point on defending jackd from useless junk.
I just want to make example more useful and up to date, but keep it simple.
And started this patch in reply to recent discussion about streaming playback.


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Kjetil S. Matheussen
In reply to this post by Dmitry Baikov

"Dmitry Baikov":
> Opps, I missed it completely.
> Took a look, and may say jack_capture is twice as large and still does
> not compare: it uses cond/mutex and does not keep timing right. I
> offer my patch for it also :)
>

Thanks for showing the use of semaphores. The syntax for semaphores looks
much nicer, so in jack_capture, maybe I'll abstract them both into
something else dependent on glibc version.

Timing is kept right in jack_capture. If you look closer into the
code, you'll see that the ringbuffer is at least twice as big as
the number of allocated blocks. That means, that if the ringbuffer
is empty, which is the only time the timing is not kept right, the
machine is struggling so hard anyway that its probably best if you
stop the recording. Its extremely unlikely to happen, and if it does,
a warning will be printed to the terminal.

jack_capture is twice as large as capture_client because it has more
functionality, is more cpu-efficient and handles underruns properly (yes,
it does). If you strip away the advanced buffer handling and the
reconnection stuff, it will be about the same size. Here is a version
without the reconnection stuff:
http://ccrma.stanford.edu/~kjetil/jackrec.c (not finished, but almost)

I started working on this one before your post, because I thought about
backporting it to jack (strange coincidence), but I'm not so sure its a
better example than capture_client.



> Anyway, capture_client resides in examples dir for a purpose: it's a
> _simple_ enough and short example how to write _good_ jack clients. It
> touches very important topics: communication of process callback and
> worker thread (simple message queue, which is nonblocking for
> rt-thread, but blocking for worker-thread) and fault recovery (which
> is often overlooked by app authors).
>

Sure, but when you write one frame at the time, your cpu usage will be
very high. I'm not so sure your version is simpler to read than mine.
Maybe a little bit, but the original capture_client.c is perhaps the
simplest one... And the original version also has fault recovery, although
its an extremely suicidal approach (deleting the file in case of overrun
or file-error).



-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

melanie-36
Hi,

why this?

  // Filename
  {
    if(filename==NULL){
      int try=0;
      filename=calloc(1,5000);
      for(;;){
        const char
*temp[20]={"jack_capture_%d.wav","jack_capture_%01d.wav","jack_capture_%02d.wav","jack_capture_%03d.wav",
                       
"jack_capture_%04d.wav","jack_capture_%05d.wav","jack_capture_%06d.wav","jack_capture_%07d.wav",
                              "jack_capture_%08d.wav","jack_capture_%09d.wav"};
        sprintf(filename,temp[leading_zeros+1],++try);
        if(access(filename,F_OK)) break;
      }
    }
  }


I know of no printf implementation that doesn't support variable
field width specifiers, of the form

sprintf(..., "jack_capture_%0*d.wav", leading_zeros, ++try);

Melanie


Kjetil S. Matheussen wrote:

> "Dmitry Baikov":
>> Opps, I missed it completely.
>> Took a look, and may say jack_capture is twice as large and still does
>> not compare: it uses cond/mutex and does not keep timing right. I
>> offer my patch for it also :)
>>
>
> Thanks for showing the use of semaphores. The syntax for semaphores looks
> much nicer, so in jack_capture, maybe I'll abstract them both into
> something else dependent on glibc version.
>
> Timing is kept right in jack_capture. If you look closer into the
> code, you'll see that the ringbuffer is at least twice as big as
> the number of allocated blocks. That means, that if the ringbuffer
> is empty, which is the only time the timing is not kept right, the
> machine is struggling so hard anyway that its probably best if you
> stop the recording. Its extremely unlikely to happen, and if it does,
> a warning will be printed to the terminal.
>
> jack_capture is twice as large as capture_client because it has more
> functionality, is more cpu-efficient and handles underruns properly (yes,
> it does). If you strip away the advanced buffer handling and the
> reconnection stuff, it will be about the same size. Here is a version
> without the reconnection stuff:
> http://ccrma.stanford.edu/~kjetil/jackrec.c (not finished, but almost)
>
> I started working on this one before your post, because I thought about
> backporting it to jack (strange coincidence), but I'm not so sure its a
> better example than capture_client.
>
>
>
>> Anyway, capture_client resides in examples dir for a purpose: it's a
>> _simple_ enough and short example how to write _good_ jack clients. It
>> touches very important topics: communication of process callback and
>> worker thread (simple message queue, which is nonblocking for
>> rt-thread, but blocking for worker-thread) and fault recovery (which
>> is often overlooked by app authors).
>>
>
> Sure, but when you write one frame at the time, your cpu usage will be
> very high. I'm not so sure your version is simpler to read than mine.
> Maybe a little bit, but the original capture_client.c is perhaps the
> simplest one... And the original version also has fault recovery, although
> its an extremely suicidal approach (deleting the file in case of overrun
> or file-error).
>
>
>
> -------------------------------------------------------------------------
> Using Tomcat but need to do more? Need to support web services, security?
> Get stuff done quickly with pre-integrated technology to make your job easier
> Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
> http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
> _______________________________________________
> Jackit-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/jackit-devel
>
>


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Dmitry Baikov
In reply to this post by Kjetil S. Matheussen
On 7/16/06, Kjetil S. Matheussen <[hidden email]> wrote:
> Timing is kept right in jack_capture. If you look closer into the
Oops, I missed the thing :( (at first I looked into 0.2.3).

Yes, jackrec is far better example in many ways (including sf_write
buffering) :)

> I started working on this one before your post, because I thought about
> backporting it to jack (strange coincidence), but I'm not so sure its a
> better example than capture_client.

Ok, I give up :)
Anyway my efforts were useful for me myself atleast :)


Thanks for you replies :)
Marking thread as closed.

Best regards,
Dmitry.


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Kjetil S. Matheussen
On Sun, 16 Jul 2006, Dmitry Baikov wrote:

> On 7/16/06, Kjetil S. Matheussen <[hidden email]> wrote:
>> Timing is kept right in jack_capture. If you look closer into the
> Oops, I missed the thing :( (at first I looked into 0.2.3).
>
> Yes, jackrec is far better example in many ways (including sf_write
> buffering) :)
>

Well, I really liked your idea of putting the block with number of
underruns and non-underruns into the ringbuffer. If using the
jack_ringbuffer_get_read/write_vector functions, it eliminates the use of
external allocated buffers and without having to read and write one frame
at the time to disk and ringbuffer, plus that it completely eliminates the
possibility of not keeping the timing and that it use less memory. I think
I'll change jack_capture to use your block header.



-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Kjetil S. Matheussen
In reply to this post by Dmitry Baikov

Melanie
>
>
> I know of no printf implementation that doesn't support variable
> field width specifiers, of the form
>
> sprintf(..., "jack_capture_%0*d.wav", leading_zeros, ++try);
>

Thanks, its included in the latest version.




-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Erik de Castro Lopo-10
In reply to this post by melanie-36
Melanie wrote:

> I know of no printf implementation that doesn't support variable
> field width specifiers, of the form
>
> sprintf(..., "jack_capture_%0*d.wav", leading_zeros, ++try);

Please don't use sprintf anywhere. The function is inherently unsafe
and has led to huge number of buffer overflows. Yes I know it sprintf
can, in some circumstances be guaranteed safe, but it will still be
flagged as a potential problem by tools like Flawfinder.

Use snprintf instead.

Erik
--
+-----------------------------------------------------------+
  Erik de Castro Lopo
+-----------------------------------------------------------+
Pastafarianism : http://www.venganza.org/
The intelligent alternative to 'Intelligent Design'.


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Kjetil S. Matheussen
In reply to this post by Dmitry Baikov

Erik de Castro Lopo:

> Melanie wrote:
>
> > I know of no printf implementation that doesn't support variable
> > field width specifiers, of the form
> >
> > sprintf(..., "jack_capture_%0*d.wav", leading_zeros, ++try);
>
> Please don't use sprintf anywhere. The function is inherently unsafe
> and has led to huge number of buffer overflows. Yes I know it sprintf
> can, in some circumstances be guaranteed safe, but it will still be
> flagged as a potential problem by tools like Flawfinder.
>
> Use snprintf instead.

Oh, come on. Saying "don't use sprintf anywhere" is just as stupid as
saying "never use goto" or "never use scanf". We are not first year
computer science students, you don't have to write statements
like that. Flawfinder sounds like a program I won't use.




-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Dmitry Baikov
> > Use snprintf instead.
>
> Oh, come on. Saying "don't use sprintf anywhere" is just as stupid as
> saying "never use goto" or "never use scanf". We are not first year
> computer science students, you don't have to write statements
> like that. Flawfinder sounds like a program I won't use.

Wrong examples: "goto" is different, you can't jump to the middle of
instruction. Scanf args are validated by gcc.

Erik is absolutely right. Please, don't create potential security hole.
sprintf is one of the biggest mistakes in libc. snprintf is how it
should be done from the beginning.

Even if we aren't first year cs students we are still people. And
still make stupid mistakes.

Regards,

Dmitry.


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Kjetil S. Matheussen
On Mon, 17 Jul 2006, Dmitry Baikov wrote:

>> > Use snprintf instead.
>>
>> Oh, come on. Saying "don't use sprintf anywhere" is just as stupid as
>> saying "never use goto" or "never use scanf". We are not first year
>> computer science students, you don't have to write statements
>> like that. Flawfinder sounds like a program I won't use.
>
> Wrong examples: "goto" is different, you can't jump to the middle of
> instruction. Scanf args are validated by gcc.
>
> Erik is absolutely right. Please, don't create potential security hole.
> sprintf is one of the biggest mistakes in libc. snprintf is how it
> should be done from the beginning.
>
> Even if we aren't first year cs students we are still people. And
> still make stupid mistakes.
>

sprintf is a cleaner command than snprintf, so code with sprintf
is easier to read than snprintf. I think that is a lot more important
factor to avoid bugs.

Besides, it seems like you think that by using sprintf, you automatically
create a potential security hole. Thats wrong, its very easy to make
secure sprintf calls.



-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Erik de Castro Lopo-10
Kjetil S. Matheussen wrote:

> sprintf is a cleaner command than snprintf, so code with sprintf
> is easier to read than snprintf. I think that is a lot more important
> factor to avoid bugs.

I disagree. If I am auditing code and I see an sprintf I have evaluate
the surrounding code very carefully to figure out if that particular
instance of sprintf is safe or not. With snprintf I know that if the
size parameter and the buffer size are the same then there can be no
buffer overrun.

In my own code I often use this:

    snprintf (buffer, sizeof (buffer), ....) ;

That is impossible to get wrong and anyone reading the code can
immediately see that there will be no overrun of the buffer. Now
look at sprintf:

    sprintf (buffer, .....) ;

Ooops, not enough information. You have to look at the format string
and figure out the length of the buffer before you can even begin to
guage if its safe.

Programming is hard. Its especially hard in a languages like C and
C++ where its so easy to shoot oneself in the foot. Programmers
should grab every oportunity they can to make their software better
and the sprintf/snprintf decision should be a no brainer.

Erik
--
+-----------------------------------------------------------+
  Erik de Castro Lopo
+-----------------------------------------------------------+
The Religion of Peace:
http://news.bbc.co.uk/2/hi/asia-pacific/4387604.stm

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Paul Davis
On Tue, 2006-07-18 at 06:50 +1000, Erik de Castro Lopo wrote:

   [ amen brother ]

in addition, using snprintf highlights a programming practice that can
otherwise be left in place but should not be:

        whatever_type some_function_that_writes_to_a_char_buf (char* buf) {
          ,,,,
          snprintf (buf, [ hmmm, what to put here ... ]


it forces you to think about how the size information gets passed
around, and either come up with a shared size system, or pass it
explicitily or ... it doesn't matter, but it forces a *solution* on
you. the code then becomes safe against future changes in design.

--p




-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel
Reply | Threaded
Open this post in threaded view
|

Re: capture_client modernization, a call for review

Kjetil S. Matheussen
In reply to this post by Dmitry Baikov

Erik de Castro Lopo:

>> sprintf is a cleaner command than snprintf, so code with sprintf
>> is easier to read than snprintf. I think that is a lot more important
>> factor to avoid bugs.
>
>I disagree. If I am auditing code and I see an sprintf I have evaluate
>the surrounding code very carefully to figure out if that particular
>instance of sprintf is safe or not. With snprintf I know that if the
>size parameter and the buffer size are the same then there can be no
>buffer overrun.
>
>In my own code I often use this:
>
>    snprintf (buffer, sizeof (buffer), ....) ;
>
>That is impossible to get wrong and anyone reading the code can
>immediately see that there will be no overrun of the buffer. Now
>look at sprintf:
>
>    sprintf (buffer, .....) ;
>
>Ooops, not enough information. You have to look at the format string
>and figure out the length of the buffer before you can even begin to
>guage if its safe.

Well, if snprintf makes you feel safer, than thats the most important.
But the buffer needs to be large enough anyway, snprintf doesn't ensure
that.

I also wonder if you misunderstand my point or something.
So, which of the following two examples do you think is the easiest to
read and quickest to determine its safety:

buffer=malloc(5000);
sprintf(buffer,"Number_of_mails=%d\n",number_of_mails);

Or:

buffer=malloc(5000);
snprintf(buffer,sizeof(buffer),"Number_of_mails=%d\n",number_of_mails);


For me, the answer is obvious: The first one, because its simpler
and that the buffer is obviously large enough. Maybe thats your problem,
you use too small buffers to immediately see that they are large
enough?


>Programming is hard. Its especially hard in a languages like C and
>C++ where its so easy to shoot oneself in the foot. Programmers
>should grab every oportunity they can to make their software better
>and the sprintf/snprintf decision should be a no brainer.

I think its stupid. But we must find a style we feel secure about
and is able to handle. So, although I think your arguments are strange,
I don't dissagree that if it works for you, you should do it.

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys -- and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
Jackit-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/jackit-devel