CallModel: the "removeMedia" function can behave incorrectly if the user doesn't have a camera
When a user stops sharing their screen in a 1:1 call, the CallModel::removeMedia
function is called and constructs a new list of media that it then sends to the daemon as part of a media change request. The expected behavior is that the new media list will be the same as the old one, except that the screen share will have been removed and replaced by the user's camera. The following line of code is responsible for adding the camera to the new media list (proposedList
):
proposedList = getProposed(proposedList,
callInfo->id,
pimpl_->lrc.getAVModel().getCurrentVideoCaptureDevice(),
MediaRequestType::CAMERA,
muteCamera);
If the user doesn't have a camera, then getCurrentVideoCaptureDevice
will return an empty string. In this case, the intended behavior of getProposed
seems to be that it will still add a media of type VIDEO
to the list, but its SOURCE
attribute will be NONE
(an empty string) instead of a camera's URI:
case MediaRequestType::CAMERA: {
// Camera device
resource = not source.isEmpty() ? QString("%1%2%3")
.arg(libjami::Media::VideoProtocolPrefix::CAMERA)
.arg(sep)
.arg(source)
: libjami::Media::VideoProtocolPrefix::NONE;
break;
}
[...]
MapStringString videoMediaAttribute = {{MediaAttributeKey::MEDIA_TYPE,
MediaAttributeValue::VIDEO},
{MediaAttributeKey::ENABLED, TRUE_STR},
{MediaAttributeKey::MUTED, mute ? TRUE_STR : FALSE_STR},
{MediaAttributeKey::SOURCE, resource},
{MediaAttributeKey::LABEL, vlabel}};
However, there are two independent bugs which make this not work as intended:
- The
getProposed
function returns an empty list if itssource
argument is empty. This bug goes back to the commit in which the function was first introduced: https://review.jami.net/c/jami-client-qt/+/26038 Making sure thatsource
is nonempty does appear to be necessary when adding a media to a call (see https://review.jami.net/c/jami-client-qt/+/23754), but this is already done inCallModel::addMedia
. - The
SIPCall::updateMediaStream
function in the daemon doesn't actually update the stream's old media source if the new media source is empty (see https://review.jami.net/c/jami-daemon/+/18835), which causes issues even if the first bug above is fixed (in particular, theSIPCall::currentMediaList
function will return an incorrect media list in which the screen share still appears after it should have been removed).