[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-multimedia
Subject:    Re: recording stream
From:       Stefan Westerfeld <stefan () space ! twc ! de>
Date:       2001-10-26 16:39:44
[Download RAW message or body]

   Hi!

On Sun, Oct 21, 2001 at 09:29:56PM +0200, Matthias Kretz wrote:
> I finally got recording from aRts working. I did a lot of stuff so let me sum 
> up:
> 
> - A play stream works using the ByteSoundProducer interface. Therefor I 
> created a ByteSoundReceiver.

Good.

> - Now the ByteSoundProducer was attached to the soundserver using 
> SimpleSoundServer->attach(ByteSoundProducer). To make it usable for me I 
> created another interface ByteSoundStream that is used for both the 
> ByteSoundProducer and ByteSoundReceiver. So I changed the method to 
> SimpleSoundServer->attach(ByteSoundStream). This is, of course, not binary 
> compatible. I don't know about wire compatibility (I only know that noatun 
> 1.2.0 (KDE 2.2) still works).
> Uhmm, while testing for WC I found out that artscat and artsdsp (from KDE 
> 2.2) won't work with my changes. But it works if I don't use the 
> ByteSoundStream interface but simply SynthModule. So the method would be 
> SimpleSoundServer->attach(SynthModule). I guess this is the better way, and 
> probably the more flexible, too. If I'm right WC will be preserved this way.

Yes, it /should/ be preserved that way. It's not exactly pretty, though,
because we'll be stuck with this completely underspecified type-unsafe attach
method. Besides, changing signatures of existing methods is at best bad for
compatibility with existing clients, at worst (with a correct MCOP
implementation that compares the signatures (see object.cc:907)) it will not
work.

I think adding new methods like this:

	void attachRecorder(...);
	void detachRecorder(...);

would be better. It's slightly asymmetric then as well, but I think it is the
best possible hack.

> - RecordStreamJob: The same as the PlaySreamJob, only using an 
> AudioToByteStream instead of ByteToAudioStream.

Ok.

> - artsdump: The same as artscat for dumping a ByteStream from aRts to a file 
> or stdout.

It needs to be called artsrec (or artsmon, if it captures the artsd overall
output), to correspond the esd name for this. I think we should make switching
easy.

> - The AudioToByteStream interface was another hurdle on the way. If I read 
> the code correctly the Resampler isn't able to convert from an aRts input to 
> a bytestream, and also can't be fitted to do that easily. So I wrote my own 
> resampler in this class. I'm not sure whether this is the right way to go and 
> I'm open for better ideas.

Reimplementing all resampling conversion code into artsd to a common code base
would be the better idea. I have been toying with the idea for some time now,
and have a few concrete thoughts how it should work. But it takes a week of
doing nothing else to unify all of them and get it right, I think ;). I might
be doing this together with Tim Janik and put the result into GSL when I have
time to do it.

> - Finally I worked on the artsc interface to support the new recording 
> capabilities.
> In artscbackend.cc there was one class Sender that would create the 
> bytestream. I copied this to a new class Stream that holds everything a 
> Receiver needs as well. Receiver and Sender inherit from Stream. Now the only 
> problem were the ArtsCApi methods that would have to be able to call methods 
> of Sender as well as of Receiver while they only receive a void*. I tried to 
> cast like this:
> 
> int stream_set(arts_stream_t stream, arts_parameter_t param, int value)
> {
>     SynthModule *_stream = (SynthModule *)stream;
>     if( _stream->_interfaceName() == "Arts::ByteStreamProducer" )
> } 
>
> But the program would just crash in _stream->_interfaceName() because 
> somewhere a dangling pointer, that should be pointing to the base class, was 
> being used. So I tried something else:
> 
> int stream_set(arts_stream_t stream, arts_parameter_t param, int value)
> {
>     Stream *_stream = (Stream *)stream;
>     if( _stream->direction() == Stream::RECORDSTREAM )
> 
> But now _stream would somehow (I have no clue, but gdb said so) be 0x0 when 
> _stream->direction() gets called. I finally got it working by doing the 
> following:
> 
> int stream_set(arts_stream_t stream, arts_parameter_t param, int value)
> {
>     Sender *_stream = (Sender *)stream;
>     _stream->stream_set(param,value);
> }
> 
> That works even if stream is a Receiver. I have a bad feeling about this, but 
> this is the only way I found to get it working.

That sounds like a horrible hack. Basically, you need to make sure that you
take exactly this out of a void* which you put in. One way to ensure this
would be:

class Stream : virtual public X, virtual public Y {
public:
	virtual void *castSender() { return 0; }
	virtual void *castReceiver() { return 0; }
};

class Sender : virtual public B, virtual public Stream {
	virtual void *castSender() { return (Sender *)this; }
};

class Receiver : virtual public C, virtual public Stream {
	virtual void *castReceiver() { return (Receiver *)this; }
};

and to put always explicitely a

(Stream*)xxx;

into the arts_stream_t. You can then explicitely ask:

	Sender *sender = (Stream*)(xxx)->castSender();
	if(sender) sender->foo();
	Receiver *receiver = (Receiver*)(xxx)->castReceiver();
	if(receiver) receiver->foo();

I mean, thats just RTTI done by hand, but it will work, always. Putting
anything into a void pointer, and taking out another type is very very
dangerous. If you use single, nonvirtual interitance only, it will work,
but otherwise, it will fail badly.

Two other remarks:

 * please adapt documentation where necessary:
    - in the IDL file
    - in the aRts handbook (kdemultimedia/doc/artsbuilder)

 * (coding style): I strongly recommend that you read the "Coding Standards"
   section in the aRts Handbook. Especially, I do recommend not using

     long m_foo;  /* wrong! ;) */

   if you have an interface { attribute long foo; }, but _foo to store it,
   thats done this way in all aRts, and if we keep it consistent, that would
   be nice. ;)

   Especially, I don't see why

-               return packetCount * packetCapacity;
+               return m_packetCount * m_packetCapacity;

   is very useful, unless you convince me that it should be changed
   everywhere in all aRts sources for all times ;). It's like the 4 spaces
   tabs thing - better that always than something else sometimes.

   Random changes between the two styles just make CVS logs harder to read.

Otherwise, the changes look quite nice.

   Cu... Stefan
-- 
  -* Stefan Westerfeld, stefan@space.twc.de (PGP!), Hamburg/Germany
     KDE Developer, project infos at http://space.twc.de/~stefan/kde *-         
_______________________________________________
kde-multimedia mailing list
kde-multimedia@mail.kde.org
http://mail.kde.org/mailman/listinfo/kde-multimedia

[prev in list] [next in list] [prev in thread] [next in thread]