[jitsi-dev] MediaDevice cannot reference MediaDeviceSession!


#1

Seb,

I'm still working on fixing the leaking of all CallDialogs and I just
noticed that you've modified AbstractMediaDevice to keep a reference
to the last MediaDeviceSession it has created - you've introduced a
MediaDeviceSession field called session and a getSession() method and
you've modified the createSession method. This is incorrect and needs
to be fixed because:

(1) A MediaDevice instance is a representation of a hardware device
instance and it is kept alive as long as the hardware device instance
is connected to the computer and accessible to Jitsi. A
MediaDeviceSession instance represents a specific use of a MediaDevice
and it has a shorter lifespan e.g. during a Call. Obviously, a single
hardware device may have multiple uses at one and the same time i.e. a
MediaDevice may create multiple MediaDeviceSessions which are alive at
one and the same time. In such a case, having a single session field
is inaccurate and/or incomplete at best.

(2) Remembering a MediaDeviceSession in a MediaDevice without clearing
it once the MediaDeviceSession is closed in a memory leak.

It's true that there are MediaDevice implementations which keep track
of their MediaDeviceSessions in Jitsi but these are special-purpose
MediaDevices such as the audio mixer and the video translator. They
are, however, different because they are created with special-purpose
methods of MediaService such as createMixer.

Regards,
Lyubomir


#2

Seb,

My suggestion for dealing with the removal of the reference from
AbstractMediaDevice to MediaDeviceSession is to move the method
MediaService#movePartialDesktopStreaming(MediaDevice, int, int) from
MediaService into VideoMediaStream. I think that it is a sound design
because:

MediaDevice/AbstractMediaDevice is like FMJ's CaptureDeviceInfo and
MediaDeviceSession is akin to FMJ's CaptureDevice/DataSource. In your
case, you want to modify a CaptureDevice/DataSource associated with a
given Call in OperationSetDesktopStreaming#movePartialDesktopStreaming(Call,
int, int). From the Call you can reach the CaptureDevice/DataSource by
going through MediaAwareCallPeer, CallPeerMediaHandler,
VideoMediaStream(Impl), (Video)MediaDeviceSession.

Please do not hesitate to contact me for further information if such a
need arises.

Best regards,
Lyubomir


#3

Hi Lyubomir,

I understand now what the problem is (from your previous mail). I will check the code and try to fix it according to your suggestion.

Thanks again for your report and your suggestion.

Best regards,

···

--
Seb

Le 13/02/12 22:01, Lyubomir Marinov a �crit :

Seb,

My suggestion for dealing with the removal of the reference from
AbstractMediaDevice to MediaDeviceSession is to move the method
MediaService#movePartialDesktopStreaming(MediaDevice, int, int) from
MediaService into VideoMediaStream. I think that it is a sound design
because:

MediaDevice/AbstractMediaDevice is like FMJ's CaptureDeviceInfo and
MediaDeviceSession is akin to FMJ's CaptureDevice/DataSource. In your
case, you want to modify a CaptureDevice/DataSource associated with a
given Call in OperationSetDesktopStreaming#movePartialDesktopStreaming(Call,
int, int). From the Call you can reach the CaptureDevice/DataSource by
going through MediaAwareCallPeer, CallPeerMediaHandler,
VideoMediaStream(Impl), (Video)MediaDeviceSession.

Please do not hesitate to contact me for further information if such a
need arises.

Best regards,
Lyubomir


#4

Seb, thank you very much for r9369 which removes the reference to
MediaDeviceSession from AbstractMediaDevice!

I'll also remove the method AbstractMediaDevice#close() and its
override AudioMixerMediaDevice#close() because these methods do not
make any sense:

(1) As I mentioned in my previous e-mail in this thread, MediaDevice
is like CaptureDeviceInfo i.e. it is immutable.

(2) The use of AbstractMediaDevice#close() by
MediaStreamImpl#setDevice(MediaDevice) is incorrect because the
MediaStreamImpl owns only the MediaDeviceSession which it has created
and it has no right to close any other MediaDeviceSession.


#5

Hi Lyubomir,

Le 14/02/12 18:59, Lyubomir Marinov a �crit :

Seb, thank you very much for r9369 which removes the reference to
MediaDeviceSession from AbstractMediaDevice!

I'll also remove the method AbstractMediaDevice#close() and its
override AudioMixerMediaDevice#close() because these methods do not
make any sense:

(1) As I mentioned in my previous e-mail in this thread, MediaDevice
is like CaptureDeviceInfo i.e. it is immutable.

(2) The use of AbstractMediaDevice#close() by
MediaStreamImpl#setDevice(MediaDevice) is incorrect because the
MediaStreamImpl owns only the MediaDeviceSession which it has created
and it has no right to close any other MediaDeviceSession.

IIRC, I add this method to close properly all mediadevicesessions prior to a "on the fly" media device change while in a conference call, to have correct audio levels.

Best regards,

···

--
Seb