Skip to content

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:

  1. The getProposed function returns an empty list if its source 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 that source 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 in CallModel::addMedia.
  2. 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, the SIPCall::currentMediaList function will return an incorrect media list in which the screen share still appears after it should have been removed).