[sip-comm-dev] Call Recording


#1

Hi all,

With the help of Lubomir's explanations I managed understand the
general structure of neomedia and add call recording to conference
calls. Since one-to-one calls are different, it doesn't work for those
yet. Here's basically what I did:
Created a new Recorder interface and implementation that has start and
stop methods and is created by MediaService from inside
MediaAwareCall. This Recorder is provided with a MediaDeviceSession.
The start() method then obtains the output datasource from the session
and hands it over to DataSink which does the writing to the file. The
stop() method calls session.close(). I think that it's not possible to
reuse the session, so every time the recording starts a new session
is created.
I also had to override a couple of methods in MediaDeviceSession:
startProcessorInAccordWithDirection and processorControllerUpdate.
startProcessorInAccordWithDirection() I need to override because it
has a "startedDirection.allowsSending()" condition and my newly
created session has an inactive direction and I'm not sure if it would
be correct to change it. processorControllerUpdate() I override
because I set MPEG_AUDIO as the processor's content descriptor, the
original was RAW_RTP.
If the hold or mute buttons are pressed during recording, only the
microphone output is recorded. This behavior seems logical to me.
I'm not sure about where to store the recorded files and how to name
them. How should the UI for managing look like? Also should I focus on
the UI or on one-to-one calls first?

Cheers,
Dmitri

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#2

Created a new Recorder interface and implementation that has start and
stop methods and is created by MediaService from inside
MediaAwareCall. This Recorder is provided with a MediaDeviceSession.
The start() method then obtains the output datasource from the session
and hands it over to DataSink which does the writing to the file. The
stop() method calls session.close(). I think that it's not possible to
reuse the session, so every time the recording starts a new session
is created.

Hm... I think I need the code in order to clearly understand what
you're talking about.

I also had to override a couple of methods in MediaDeviceSession:
startProcessorInAccordWithDirection and processorControllerUpdate.
startProcessorInAccordWithDirection() I need to override because it
has a "startedDirection.allowsSending()" condition and my newly
created session has an inactive direction and I'm not sure if it would
be correct to change it.

If it's the deviceSession of a MediaStream, I wouldn't expect you to
have the right to change its startedDirection. Anyway, if you send us
a patch with the code, I may be able to tell you more about it.

processorControllerUpdate() I override
because I set MPEG_AUDIO as the processor's content descriptor, the
original was RAW_RTP.

My comments on startProcessorInAccordWithDirection above apply here as well.

Also should I focus on the UI or on one-to-one calls first?

Unless you have an idea how to approach one-to-one calls, my advice
would be to figure out the UI because I intend to move functionality
from the conference calls to the one-to-one calls in order to be able
to enable call recording. But I need to see what you need from the
conference calls in order to decide what is to be moved.

···

On Thu, Jul 22, 2010 at 1:39 PM, Dmitri Melnikov <dmitri807@gmail.com> wrote:

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#3

Hi,
You're right, it's easier just to read the code.
The record button has the same icon as the mute button, but the
tooltip is different. And the recorded call is saved to "newfile.wav"
in the current directory.

Dmitri

recording.patch (18.5 KB)

···

On Thu, Jul 22, 2010 at 1:53 PM, Lubomir Marinov <lubo@sip-communicator.org> wrote:

On Thu, Jul 22, 2010 at 1:39 PM, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Created a new Recorder interface and implementation that has start and
stop methods and is created by MediaService from inside
MediaAwareCall. This Recorder is provided with a MediaDeviceSession.
The start() method then obtains the output datasource from the session
and hands it over to DataSink which does the writing to the file. The
stop() method calls session.close(). I think that it's not possible to
reuse the session, so every time the recording starts a new session
is created.

Hm... I think I need the code in order to clearly understand what
you're talking about.

I also had to override a couple of methods in MediaDeviceSession:
startProcessorInAccordWithDirection and processorControllerUpdate.
startProcessorInAccordWithDirection() I need to override because it
has a "startedDirection.allowsSending()" condition and my newly
created session has an inactive direction and I'm not sure if it would
be correct to change it.

If it's the deviceSession of a MediaStream, I wouldn't expect you to
have the right to change its startedDirection. Anyway, if you send us
a patch with the code, I may be able to tell you more about it.

processorControllerUpdate() I override
because I set MPEG_AUDIO as the processor's content descriptor, the
original was RAW_RTP.

My comments on startProcessorInAccordWithDirection above apply here as well.

Also should I focus on the UI or on one-to-one calls first?

Unless you have an idea how to approach one-to-one calls, my advice
would be to figure out the UI because I intend to move functionality
from the conference calls to the one-to-one calls in order to be able
to enable call recording. But I need to see what you need from the
conference calls in order to decide what is to be moved.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#4

Hi Dmitri,

Thank you for this complete report.

Cheers,

Ben

···

On Thu, Jul 22, 2010 at 13:25, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Hi,
You're right, it's easier just to read the code.
The record button has the same icon as the mute button, but the
tooltip is different. And the recorded call is saved to "newfile.wav"
in the current directory.

Dmitri

On Thu, Jul 22, 2010 at 1:53 PM, Lubomir Marinov > <lubo@sip-communicator.org> wrote:

On Thu, Jul 22, 2010 at 1:39 PM, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Created a new Recorder interface and implementation that has start and
stop methods and is created by MediaService from inside
MediaAwareCall. This Recorder is provided with a MediaDeviceSession.
The start() method then obtains the output datasource from the session
and hands it over to DataSink which does the writing to the file. The
stop() method calls session.close(). I think that it's not possible to
reuse the session, so every time the recording starts a new session
is created.

Hm... I think I need the code in order to clearly understand what
you're talking about.

I also had to override a couple of methods in MediaDeviceSession:
startProcessorInAccordWithDirection and processorControllerUpdate.
startProcessorInAccordWithDirection() I need to override because it
has a "startedDirection.allowsSending()" condition and my newly
created session has an inactive direction and I'm not sure if it would
be correct to change it.

If it's the deviceSession of a MediaStream, I wouldn't expect you to
have the right to change its startedDirection. Anyway, if you send us
a patch with the code, I may be able to tell you more about it.

processorControllerUpdate() I override
because I set MPEG_AUDIO as the processor's content descriptor, the
original was RAW_RTP.

My comments on startProcessorInAccordWithDirection above apply here as well.

Also should I focus on the UI or on one-to-one calls first?

Unless you have an idea how to approach one-to-one calls, my advice
would be to figure out the UI because I intend to move functionality
from the conference calls to the one-to-one calls in order to be able
to enable call recording. But I need to see what you need from the
conference calls in order to decide what is to be moved.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#5

Hello Dmitri,

Re: recording.patch, I currently have the following to share with you:

- I can hardly wait to a see a contribution from you outside GSoC so
that I can ask you whether you want to become a committer.

- RecorderImpl needs a DataSource and it takes a whole
MediaDeviceSession which is kind of... heavyweight. Besides, a
MediaDeviceSession is something that a MediaStream uses from its
device so it want to have control over it, it wants to add and remove
playback streams - these are not the kind of pieces of functionality
that'd strike me as necessary to the call recorder. Could you please
give the advantages and disadvantages of using DataSource and
MediaDeviceSession?

- I'm not particularly comfortable with MediaAwareCall#getRecorder
using getDefaultDevice. I admit it may be a moot point but it doesn't
express in general that it will use the same device as the call.
Anyway, I'll have to think more about an approach I feel comfortable
with.

- I don't quite understand why MediaAwareCall deals with Recorder and
OperationSetBasicTelephony doesn't. I mean why don't we work with
Recorder in the abstract implementation of OperationSetBasicTelephony
instead of in the very protocol-specific implementations? It just
seems weird to me because it's asymmetric.

- At a later point in the development, we may want to make it possible
to save the call records in various audio formats. For example, I may
prefer to keep my call records in a compressed format instead of .wav.
Then we may have to add a format property to Recorder.

- Just for the record, recording.patch is missing RecordButton.java.

Regards,
Lubomir

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#6

Hi Lubomir,

Thanks for taking the time to look at this!

Hello Dmitri,

Re: recording.patch, I currently have the following to share with you:

- I can hardly wait to a see a contribution from you outside GSoC so
that I can ask you whether you want to become a committer.

- RecorderImpl needs a DataSource and it takes a whole
MediaDeviceSession which is kind of... heavyweight. Besides, a
MediaDeviceSession is something that a MediaStream uses from its
device so it want to have control over it, it wants to add and remove
playback streams - these are not the kind of pieces of functionality
that'd strike me as necessary to the call recorder. Could you please
give the advantages and disadvantages of using DataSource and
MediaDeviceSession?

I agree that RecorderImpl only needs 2 methods from the
MediaDeviceSession: getOutputDataSource() and close(). Initially when
I started I tried to only pass an AudioMixerMediaDevice to the
Recorder and call it's createOutputDataSource() to get the datasource.
But I could not use this datasource to create a DataSink (exception
occurs). I think it only accepts a datasource that comes directly from
the Processor's getOutputDataSource(). So I began to write code for
creating a Processor, when I noticed that everything I needed was
already done in the MediaDeviceSession. The methods of
MediaDeviceSession call each other until they get back to the same
AudioMixerMediaDevice.createOutputDataSource(). Additionally these
method calls wrap the datasource in MutePushBufferDataSource and pass
it to the Processor and start it.
So the bottom line is that I used MediaDeviceSession because
everything what I wanted to do was already implemented there (creating
and starting the Processor with the correct datasource).

- I'm not particularly comfortable with MediaAwareCall#getRecorder
using getDefaultDevice. I admit it may be a moot point but it doesn't
express in general that it will use the same device as the call.
Anyway, I'll have to think more about an approach I feel comfortable
with.

- I don't quite understand why MediaAwareCall deals with Recorder and
OperationSetBasicTelephony doesn't. I mean why don't we work with
Recorder in the abstract implementation of OperationSetBasicTelephony
instead of in the very protocol-specific implementations? It just
seems weird to me because it's asymmetric.

Initially I looked at how setMute was done and it calls setMute on the call, so
I did the same. But there's nothing that prevents to put the code inside
the abstract implementation of OperationSetBasicTelephony if it has access
to MediaService.

- At a later point in the development, we may want to make it possible
to save the call records in various audio formats. For example, I may
prefer to keep my call records in a compressed format instead of .wav.
Then we may have to add a format property to Recorder.

Actually the saved call is in MPEG 1 Audio, Layer 2 (as ubuntu shows
it). It's just that the filename has a .wav extension (should be mp2).
The output format is determined by processor.setContentDescriptor() I
think.

- Just for the record, recording.patch is missing RecordButton.java.

Sorry about that, I attached it.

Cheers,
Dmitri

recordButton.patch (3.94 KB)

···

On Fri, Jul 23, 2010 at 4:52 PM, Lubomir Marinov <lubo@sip-communicator.org> wrote:


#7

Initially when
I started I tried to only pass an AudioMixerMediaDevice to the
Recorder and call it's createOutputDataSource() to get the datasource.
But I could not use this datasource to create a DataSink (exception
occurs).
I think it only accepts a datasource that comes directly from
the Processor's getOutputDataSource().

Please report exceptions. For what it's worth, AudioMixer should
return DataSource/CaptureDevice implementations which are compatible
with any other JMF DataSource/CaptureDevice. Besides, Processor
doesn't return a DataSource which is totally different and
Processor-specific than AudioMixer's and AudioMixer uses Processor as
well.

So I began to write code for
creating a Processor, when I noticed that everything I needed was
already done in the MediaDeviceSession. The methods of
MediaDeviceSession call each other until they get back to the same
AudioMixerMediaDevice.createOutputDataSource(). Additionally these
method calls wrap the datasource in MutePushBufferDataSource and pass
it to the Processor and start it.
So the bottom line is that I used MediaDeviceSession because
everything what I wanted to do was already implemented there (creating
and starting the Processor with the correct datasource).

On the subject of dealing with the Processor, MediaDeviceSession is
not implemented by carving its code into stone so please feel free to
offer a design to refactor the Processor code out of it into a module
which can be used by MediaDeviceSession and the Recoder
implementation.

As to the mute support, I understand that MediaDeviceSession will give
it to you but I fail to see how it's going to make any difference to
the DataSource (i.e. MediaDeviceSession) that you create for the
Recorder. Admittedly, there was a recent rethinking of the mute
functionality and the way it is applied. But as far as I remember and
understand the code from a quick look, the MediaStream will mute its
DataSource/MediaDeviceSession but not the AudioMixerMediaDeviceSession
so that your DataSource/MediaDeviceSession gets muted... Could you
please elaborate on the subject?

Initially I looked at how setMute was done and it calls setMute on the call, so
I did the same. But there's nothing that prevents to put the code inside
the abstract implementation of OperationSetBasicTelephony if it has access
to MediaService.

Media-aware protocol classes in .services were added after the
complete development of the mute functionality. While the rest of the
code may have not been migrated to better fit the refined
architecture, it doesn't mean we'll continue writing according to the
earlier version. Please try to write your new code in accord with the
latest architecture.

Actually the saved call is in MPEG 1 Audio, Layer 2 (as ubuntu shows
it). It's just that the filename has a .wav extension (should be mp2).
The output format is determined by processor.setContentDescriptor() I
think.

I was speaking in general, of course. If the other community members
think that it's worth it to be prepared to handle user's choice for
the output format of the call records, I don't see why we wouldn't
want to talk about it now.

···

On Fri, Jul 23, 2010 at 6:37 PM, Dmitri Melnikov <dmitri807@gmail.com> wrote:

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#8

Hi,

Initially when
I started I tried to only pass an AudioMixerMediaDevice to the
Recorder and call it's createOutputDataSource() to get the datasource.
But I could not use this datasource to create a DataSink (exception
occurs).
I think it only accepts a datasource that comes directly from
the Processor's getOutputDataSource().

Please report exceptions. For what it's worth, AudioMixer should
return DataSource/CaptureDevice implementations which are compatible
with any other JMF DataSource/CaptureDevice. Besides, Processor
doesn't return a DataSource which is totally different and
Processor-specific than AudioMixer's and AudioMixer uses Processor as
well.

It is a NoDataSinkException thrown by Manager.createDataSink:
javax.media.NoDataSinkException: Cannot find a DataSink for:
net.java.sip.communicator.impl.neomedia.conference.AudioMixingPushBufferDataSource@1abcd5e
My guess is that the processor's ContentDescriptor is wrong. I got the
same exception when processor had RAW_RTP. But maybe it's something
else.

So I began to write code for
creating a Processor, when I noticed that everything I needed was
already done in the MediaDeviceSession. The methods of
MediaDeviceSession call each other until they get back to the same
AudioMixerMediaDevice.createOutputDataSource(). Additionally these
method calls wrap the datasource in MutePushBufferDataSource and pass
it to the Processor and start it.
So the bottom line is that I used MediaDeviceSession because
everything what I wanted to do was already implemented there (creating
and starting the Processor with the correct datasource).

On the subject of dealing with the Processor, MediaDeviceSession is
not implemented by carving its code into stone so please feel free to
offer a design to refactor the Processor code out of it into a module
which can be used by MediaDeviceSession and the Recoder
implementation.

I'll think about this.

As to the mute support, I understand that MediaDeviceSession will give
it to you but I fail to see how it's going to make any difference to
the DataSource (i.e. MediaDeviceSession) that you create for the
Recorder. Admittedly, there was a recent rethinking of the mute
functionality and the way it is applied. But as far as I remember and
understand the code from a quick look, the MediaStream will mute its
DataSource/MediaDeviceSession but not the AudioMixerMediaDeviceSession
so that your DataSource/MediaDeviceSession gets muted... Could you
please elaborate on the subject?

I'm not really sure how muting works exactly with respect to
MediaDeviceSession. Perhaps it really does not make a difference, some
more experimenting required here.

Initially I looked at how setMute was done and it calls setMute on the call, so
I did the same. But there's nothing that prevents to put the code inside
the abstract implementation of OperationSetBasicTelephony if it has access
to MediaService.

Media-aware protocol classes in .services were added after the
complete development of the mute functionality. While the rest of the
code may have not been migrated to better fit the refined
architecture, it doesn't mean we'll continue writing according to the
earlier version. Please try to write your new code in accord with the
latest architecture.

I just noticed that AbstractOperationSetBasicTelephony is in a package
that does not
depend on neomedia and the Recorder is in neomedia. Therefore the
default implementation
cannot be added to the abstract class without introducing this dependency.

Actually the saved call is in MPEG 1 Audio, Layer 2 (as ubuntu shows
it). It's just that the filename has a .wav extension (should be mp2).
The output format is determined by processor.setContentDescriptor() I
think.

I was speaking in general, of course. If the other community members
think that it's worth it to be prepared to handle user's choice for
the output format of the call records, I don't see why we wouldn't
want to talk about it now.

I agree, different format support would be nice to have.

Dmitri

···

On Fri, Jul 23, 2010 at 9:16 PM, Lubomir Marinov <lubo@sip-communicator.org> wrote:

On Fri, Jul 23, 2010 at 6:37 PM, Dmitri Melnikov <dmitri807@gmail.com> wrote:

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#9

With MediaAwareCall, MediaAwareCallPeer and CallPeerMediaHandler
(among the other classes in .service.protocol.media) which have
bubbled from .impl.protocol.sip into .service.protocol.media, what's
protocol-specific in the call recording functionality which is
supposed to serialize the DataSource of the neomedia into a file?

···

On Fri, Jul 23, 2010 at 11:19 PM, Dmitri Melnikov <dmitri807@gmail.com> wrote:

I just noticed that AbstractOperationSetBasicTelephony is in a package
that does not
depend on neomedia and the Recorder is in neomedia. Therefore the
default implementation
cannot be added to the abstract class without introducing this dependency.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#10

Hi all,

I made some further improvements on call recording:

- The UI is now in General and consists of a directory to save the
call to and supported formats combo box.
- The supported formats are mp2, wav, au, aiff and gsm. Default is
mp2. All of them are supported by JMF out of the box. Mp3 I think is
not supported by JMF.
- The call name is based on the current date like this:
"2010-08-02@23.39.03-confcall.mp2".
- As Lubomir pointed out, recording is not protocol specific, so I put
it in AbstractOperationSetBasicTelephony.
- Small changes and some refactoring

I also tried refactoring the Processor out of MediaDeviceSession, as
Lubomir suggested. But this turned out to be a non-trivial task. There
are many dependencies between various components and at one point it
just got too messy. I believe it's possible to do it somehow and get a
wrapper around session's Processor that's unaware of the session
itself (although must still use it's capture device), but it's not a
simple task and can potentially cause a lot of bugs. So right now I'm
still creating a whole new session for recording.

I'm attaching a patch in case someone wants to look at the code or
test it. I hope nothing is missing.

Cheers,
Dmitri

recording.patch (44.8 KB)


#11

Hello Dmitri,

First of, great work so far, I'm sure nobody would disagree on that.

I just have one small request. Can you please sync your branch with the
trunk and commit your changes there? So far, nobody asked you to do so,
so I guess I'm the weirdo, I'm sorry. The reason I'm asking you that is
because diffing between revisions and listing the history of your branch
makes it easier to follow the changes and obtain an overall picture of
your progress than having to apply patches and search discussion threads.

Cheers,
George

···

On 08/02/2010 11:08 PM, Dmitri Melnikov wrote:

Hi all,

I made some further improvements on call recording:

- The UI is now in General and consists of a directory to save the
call to and supported formats combo box.
- The supported formats are mp2, wav, au, aiff and gsm. Default is
mp2. All of them are supported by JMF out of the box. Mp3 I think is
not supported by JMF.
- The call name is based on the current date like this:
"2010-08-02@23.39.03-confcall.mp2".
- As Lubomir pointed out, recording is not protocol specific, so I put
it in AbstractOperationSetBasicTelephony.
- Small changes and some refactoring

I also tried refactoring the Processor out of MediaDeviceSession, as
Lubomir suggested. But this turned out to be a non-trivial task. There
are many dependencies between various components and at one point it
just got too messy. I believe it's possible to do it somehow and get a
wrapper around session's Processor that's unaware of the session
itself (although must still use it's capture device), but it's not a
simple task and can potentially cause a lot of bugs. So right now I'm
still creating a whole new session for recording.

I'm attaching a patch in case someone wants to look at the code or
test it. I hope nothing is missing.

Cheers,
Dmitri

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#12

Hello Dmitri,

I reviewed and tested your recording.patch and I liked it.

I wanted to commit it but I didn't agree with a few things. I thought
I'd manage to deal with them but new ones kept appearing and I finally
reverted and decided to just let you handle all of them.

- The package .service.protocol cannot reference media. Its
counterpart which can is called .service.protocol.media and is
separately distributed in protocol-media.jar. Since
OperationSetBasicTelephony is about streaming media, I think it's a
good idea to put AbstractOperationSetBasicTelephony in
protocol-media.jar (i.e. move the class in question to the package
.service.protocol.media) and allow it to access MediaAwareCall.

- AbstractOperationSetBasicTelephony cannot have a Recorder field
because it handles multiple calls. The Recorder reference has to be
specific to the Call instance. After giving
AbstractOperationSetBasicTelephony access to MediaAwareCall, I think
the Recorder can be placed in MediaAwareCall.

- The neomedia cannot reference .service.protocol (I believe). There's
currently a reference in SecurityEventManager but I don't think we
should introduce another reference for the purposes of the Recorder.
Recorder needs a MediaDevice with an AudioMixer, it doesn't need a
whole Call i.e. have MediaService#createRecorder() take a MediaDevice,
not a Call.

- RecordButton is a copy of an existing class. There's a pattern in
the extenders of SIPCommToggleButton so extract a new base from them
and make all of them extend it, don't copy the existing file and just
change a few lines without even changing the comments.

- We need an icon for the RecordButton.

- Tools > Options > General is too crowded already. We have to think
of a better place for the Call Recording configuration UI.

Emil also reminded me in a private conversation of the following from
the "Call Recording UI" thread:

- The configuration of the Call Recording has to allow the user to
choose whether the call records are to always go in a specific folder
or the user is to choose on a per-call basis.

- In the "always" case, the user should be able to choose a folder in
the configuration UI. The default directory shouldn't be
~/.sip-communicator/calls but rather something known such as Desktop,
My Documents (on Windows) or Documents (on Mac OS X and Linux).

- The Record button tool tip will also tell the user which is the chosen folder.

- In the per-call case, the user will have to specify the file and the
format when they press the Record button.

- A notification message is to be displayed to notify the user where
the call record is to be found. For example, at the end of the
recording.

Regards,
Lubomir

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#13

Hi George,

Hello Dmitri,

First of, great work so far, I'm sure nobody would disagree on that.

Thanks!

I just have one small request. Can you please sync your branch with the
trunk and commit your changes there? So far, nobody asked you to do so,
so I guess I'm the weirdo, I'm sorry. The reason I'm asking you that is
because diffing between revisions and listing the history of your branch
makes it easier to follow the changes and obtain an overall picture of
your progress than having to apply patches and search discussion threads.

I'm not using a branch for call recording, but working with the latest
version of the trunk, so you should have no problem with applying the
patch.

Cheers,
Dmitri

···

On Tue, Aug 3, 2010 at 11:58 PM, George Politis <666f6f@gmail.com> wrote:

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#14

Hey guys,

a couple of quick comments inline:

На 06.08.10 21:28, Lubomir Marinov написа:

Hello Dmitri,

I reviewed and tested your recording.patch and I liked it.

I wanted to commit it but I didn't agree with a few things. I thought
I'd manage to deal with them but new ones kept appearing and I finally
reverted and decided to just let you handle all of them.

- The package .service.protocol cannot reference media. Its
counterpart which can is called .service.protocol.media and is
separately distributed in protocol-media.jar. Since
OperationSetBasicTelephony is about streaming media, I think it's a
good idea to put AbstractOperationSetBasicTelephony in
protocol-media.jar (i.e. move the class in question to the package
.service.protocol.media) and allow it to access MediaAwareCall.

- AbstractOperationSetBasicTelephony cannot have a Recorder field
because it handles multiple calls. The Recorder reference has to be
specific to the Call instance. After giving
AbstractOperationSetBasicTelephony access to MediaAwareCall, I think
the Recorder can be placed in MediaAwareCall.

- The neomedia cannot reference .service.protocol (I believe).

Confirmed!

There's currently a reference in SecurityEventManager

Oops. This one must have slipped by. We should try to get rid of it.

Cheers,
Emil

···

but I don't think we
should introduce another reference for the purposes of the Recorder.
Recorder needs a MediaDevice with an AudioMixer, it doesn't need a
whole Call i.e. have MediaService#createRecorder() take a MediaDevice,
not a Call.

- RecordButton is a copy of an existing class. There's a pattern in
the extenders of SIPCommToggleButton so extract a new base from them
and make all of them extend it, don't copy the existing file and just
change a few lines without even changing the comments.

- We need an icon for the RecordButton.

- Tools > Options > General is too crowded already. We have to think
of a better place for the Call Recording configuration UI.

Emil also reminded me in a private conversation of the following from
the "Call Recording UI" thread:

- The configuration of the Call Recording has to allow the user to
choose whether the call records are to always go in a specific folder
or the user is to choose on a per-call basis.

- In the "always" case, the user should be able to choose a folder in
the configuration UI. The default directory shouldn't be
~/.sip-communicator/calls but rather something known such as Desktop,
My Documents (on Windows) or Documents (on Mac OS X and Linux).

- The Record button tool tip will also tell the user which is the chosen folder.

- In the per-call case, the user will have to specify the file and the
format when they press the Record button.

- A notification message is to be displayed to notify the user where
the call record is to be found. For example, at the end of the
recording.

Regards,
Lubomir

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net

--
Emil Ivov, Ph.D. 67000 Strasbourg,
Project Lead France
SIP Communicator
emcho@sip-communicator.org PHONE: +33.1.77.62.43.30
http://sip-communicator.org FAX: +33.1.77.62.47.31

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#15

I just have one small request. Can you please sync your branch with the
trunk and commit your changes there? So far, nobody asked you to do so,
so I guess I'm the weirdo, I'm sorry. The reason I'm asking you that is
because diffing between revisions and listing the history of your branch
makes it easier to follow the changes and obtain an overall picture of
your progress than having to apply patches and search discussion threads.

I'm not using a branch for call recording, but working with the latest
version of the trunk, so you should have no problem with applying the
patch.

I was referring to the password storage branch. I don't see anything bad
in using it for call recording as well. In any case, patches aren't that
bad :slight_smile:

Cheers,
George

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#16

Hey George,

На 03.08.10 23:33, George Politis написа:

I was referring to the password storage branch. I don't see anything bad
in using it for call recording as well. In any case, patches aren't that
bad :slight_smile:

I believe I was the one who suggested to Dmitri not to use a branch for
call recording. Given that we don't have that much time left, I thought
we'd rather skip all the hassle with pre SVN pre-1.5 merging.

I am sorry for not making this clear to everyone.

Lubo, Ben, George, let me know if this is making mentoring more difficult.

Cheers,
Emil

···

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#17

Patches are absolutely fine. I'm sorry for bringing up a pointless
subject. Keep up the good work Dmitri.

Cheers everyone,
George

···

On 08/03/2010 11:50 PM, Emil Ivov wrote:

Hey George,

На 03.08.10 23:33, George Politis написа:

I was referring to the password storage branch. I don't see anything bad
in using it for call recording as well. In any case, patches aren't that
bad :slight_smile:

I believe I was the one who suggested to Dmitri not to use a branch for
call recording. Given that we don't have that much time left, I thought
we'd rather skip all the hassle with pre SVN pre-1.5 merging.

I am sorry for not making this clear to everyone.

Lubo, Ben, George, let me know if this is making mentoring more difficult.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#18

Hi,

Just a small update on one-to-one call recording. It was actually
possible to add it by editing only one line of code! By ignoring the
conference focus condition we create an AudioMixer for all calls and
that already has recording implemented. I'm not sure how acceptable is
this solution. I'm sure Lubomir can comment more on this.

Cheers,
Dmitri

### Eclipse Workspace Patch 1.0
#P sip-communicator
Index: src/net/java/sip/communicator/service/protocol/media/MediaAwareCall.java

···

===================================================================
--- src/net/java/sip/communicator/service/protocol/media/MediaAwareCall.java (revision
7535)
+++ src/net/java/sip/communicator/service/protocol/media/MediaAwareCall.java (working
copy)
@@ -329,7 +329,7 @@
         MediaDevice device = mediaService.getDefaultDevice(mediaType,
                 mediaUseCase);

- if (MediaType.AUDIO.equals(mediaType) && isConferenceFocus())
+ if (MediaType.AUDIO.equals(mediaType))
         {
             if (conferenceAudioMixer == null)
             {

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#19

OK. But let's wrap the call recording in the case of conference calls
before we commit this solution.

···

On Thu, Aug 5, 2010 at 7:45 PM, Dmitri Melnikov <dmitri807@gmail.com> wrote:

Index: src/net/java/sip/communicator/service/protocol/media/MediaAwareCall.java
- if (MediaType.AUDIO.equals(mediaType) && isConferenceFocus())
+ if (MediaType.AUDIO.equals(mediaType))

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@sip-communicator.dev.java.net
For additional commands, e-mail: dev-help@sip-communicator.dev.java.net


#20

Hi all,

This patch deals with the changes that Lubormir talked about in his email.
I put the UI under Advanced tab and made a new bundle for it called
callrecordingconfig.jar. I also made a new record picture.
Let me know if there's anything else that needs to be changed or added.

Cheers,
Dmitri

callRecording.patch (78.7 KB)