Skip to content

Commit

Permalink
conference: identify mixed videos per streamId
Browse files Browse the repository at this point in the history 
Videos in the videoMixer_ were not easy to identify, moreover,
"sinkId" in the participants informations was the concatenation
of the confId and a URI, which is a problem if several devices
from the same account was present in the conference. Finally,
the active stream logic was dirty, with two different variables
used to identify the active stream in the mixer.

This patch introduces a streamId which is (callId_type_idx, e.g.
ca111412_video_0, ca111412_audio_2), so every video shown in the
conference are identified via a unique ID.
Active stream in the video mixer is identified by this ID, not
by the callId or pointer.

This should not change any behaviour, but prepare for multistream.

https://git.jami.net/savoirfairelinux/jami-project/-/issues/1429

Change-Id: I250dd31ad1ea92ed1fd1e94bec2f5abd311d2128
  • Loading branch information
Sébastien Blin committed Jun 22, 2022
1 parent 13ceecd commit ce645d4
Show file tree
Hide file tree
Showing 18 changed files with 222 additions and 221 deletions.
9 changes: 1 addition & 8 deletions src/client/callmanager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,14 +578,7 @@ setActiveStream(const std::string& accountId,
{
if (const auto account = jami::Manager::instance().getAccount<jami::JamiAccount>(accountId)) {
if (auto conf = account->getConference(confId)) {
// TODO, as for now the videoMixer doesn't have the streamId
if (deviceId == account->currentDeviceId() && accountUri == account->getUsername()) {
conf->setActiveStream("", state);
} else if (auto call = std::static_pointer_cast<jami::SIPCall>(
conf->getCallFromPeerID(accountUri))) {
conf->setActiveStream(call->getCallId(), state);
}
// conf->setActiveStream(streamId, state);
conf->setActiveStream(streamId, state);
} else if (auto call = account->getCall(confId)) {
if (call->conferenceProtocolVersion() == 1) {
Json::Value sinkVal;
Expand Down
114 changes: 35 additions & 79 deletions src/conference.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,14 @@ Conference::Conference(const std::shared_ptr<Account>& account)
}
auto hostAdded = false;
// Handle participants showing their video
std::unique_lock<std::mutex> lk(shared->videoToCallMtx_);
for (const auto& info : infos) {
std::string uri {};
bool isLocalMuted = false;
std::string deviceId {};
auto active = false;
if (!info.id.empty()) {
if (auto call = std::dynamic_pointer_cast<SIPCall>(getCall(info.id))) {
if (!info.callId.empty()) {
std::string callId = info.callId;
if (auto call = std::dynamic_pointer_cast<SIPCall>(getCall(callId))) {
uri = call->getPeerNumber();
isLocalMuted = call->isPeerMuted();
if (auto* transport = call->getTransport())
Expand All @@ -125,13 +125,12 @@ Conference::Conference(const std::shared_ptr<Account>& account)
std::string_view peerId = string_remove_suffix(uri, '@');
auto isModerator = shared->isModerator(peerId);
auto isHandRaised = shared->isHandRaised(deviceId);
auto isModeratorMuted = shared->isMuted(info.id);
auto sinkId = shared->getConfId() + peerId;
auto isModeratorMuted = shared->isMuted(callId);
if (auto videoMixer = shared->videoMixer_)
active = videoMixer->verifyActive(info.id); // TODO streamId
active = videoMixer->verifyActive(info.streamId);
newInfo.emplace_back(ParticipantInfo {std::move(uri),
deviceId,
std::move(sinkId),
std::move(info.streamId),
active,
info.x,
info.y,
Expand All @@ -143,30 +142,31 @@ Conference::Conference(const std::shared_ptr<Account>& account)
isModerator,
isHandRaised});
} else {
auto it = shared->videoToCall_.find(info.source);
if (it == shared->videoToCall_.end())
it = shared->videoToCall_.emplace_hint(it, info.source, std::string());
// If not local
auto isModeratorMuted = false;
if (!it->second.empty()) {
// If not local
auto streamInfo = shared->videoMixer_->streamInfo(info.source);
std::string streamId = streamInfo.streamId;
if (!streamId.empty()) {
// Retrieve calls participants
// TODO: this is a first version, we assume that the peer is not
// a master of a conference and there is only one remote
// In the future, we should retrieve confInfo from the call
// To merge layouts informations
// TODO sinkId
auto isModeratorMuted = shared->isMuted(it->second);
isModeratorMuted = shared->isMuted(streamId);
if (auto videoMixer = shared->videoMixer_)
active = videoMixer->verifyActive(it->second);
if (auto call = std::dynamic_pointer_cast<SIPCall>(getCall(it->second))) {
active = videoMixer->verifyActive(streamId);
if (auto call = std::dynamic_pointer_cast<SIPCall>(getCall(streamInfo.callId))) {
uri = call->getPeerNumber();
isLocalMuted = call->isPeerMuted();
if (auto* transport = call->getTransport())
deviceId = transport->deviceId();
}
} else {
streamId = sip_utils::streamId("", 0, MediaType::MEDIA_VIDEO);
if (auto videoMixer = shared->videoMixer_)
active = videoMixer->verifyActive(streamId);
}
std::string_view peerId = string_remove_suffix(uri, '@');
// TODO (another patch): use deviceId instead of peerId as specified in protocol
auto isModerator = shared->isModerator(peerId);
if (uri.empty() && !hostAdded) {
hostAdded = true;
Expand All @@ -175,12 +175,9 @@ Conference::Conference(const std::shared_ptr<Account>& account)
isLocalMuted = shared->isMediaSourceMuted(MediaType::MEDIA_AUDIO);
}
auto isHandRaised = shared->isHandRaised(deviceId);
auto sinkId = shared->getConfId() + peerId;
if (auto videoMixer = shared->videoMixer_)
active |= videoMixer->verifyActive(info.source);
newInfo.emplace_back(ParticipantInfo {std::move(uri),
deviceId,
std::move(sinkId),
std::move(streamId),
active,
info.x,
info.y,
Expand All @@ -197,7 +194,6 @@ Conference::Conference(const std::shared_ptr<Account>& account)
newInfo.h = videoMixer->getHeight();
newInfo.w = videoMixer->getWidth();
}
lk.unlock();
if (!hostAdded) {
ParticipantInfo pi;
pi.videoMuted = true;
Expand All @@ -218,10 +214,8 @@ Conference::Conference(const std::shared_ptr<Account>& account)
});
parser_.onRaiseHand(
[&](const auto& deviceId, bool state) { setHandRaised(deviceId, state); });
parser_.onSetActiveStream([&](const auto& accountUri, const auto& deviceId, bool state) {
// TODO replace per streamId
if (auto call = getCallWith(accountUri, deviceId))
setHandRaised(call->getCallId(), state);
parser_.onSetActiveStream([&](const auto& streamId, bool state) {
setActiveStream(streamId, state);
});
parser_.onMuteStreamAudio
(
Expand Down Expand Up @@ -616,11 +610,12 @@ Conference::handleMediaChangeRequest(const std::shared_ptr<Call>& call,

#ifdef ENABLE_VIDEO
// If the new media list has video, remove the participant from audioonlylist.
if (MediaAttribute::hasMediaType(MediaAttribute::buildMediaAttributesList(remoteMediaList,
false),
MediaType::MEDIA_VIDEO)) {
if (videoMixer_)
videoMixer_->removeAudioOnlySource(call->getCallId());
if (videoMixer_ && MediaAttribute::hasMediaType(
MediaAttribute::buildMediaAttributesList(remoteMediaList, false),
MediaType::MEDIA_VIDEO)) {
auto callId = call->getCallId();
videoMixer_->removeAudioOnlySource(callId,
std::string(sip_utils::streamId(callId, 0, MediaType::MEDIA_VIDEO)));
}
#endif

Expand Down Expand Up @@ -708,10 +703,8 @@ Conference::addParticipant(const std::string& participant_id)
// In conference, if a participant joins with an audio only
// call, it must be listed in the audioonlylist.
auto mediaList = call->getMediaAttributeList();
if (not MediaAttribute::hasMediaType(mediaList, MediaType::MEDIA_VIDEO)) {
if (videoMixer_) {
videoMixer_->addAudioOnlySource(call->getCallId());
}
if (videoMixer_ && not MediaAttribute::hasMediaType(mediaList, MediaType::MEDIA_VIDEO)) {
videoMixer_->addAudioOnlySource(call->getCallId(), sip_utils::streamId(call->getCallId(), 0, MediaType::MEDIA_AUDIO));
}
call->enterConference(shared_from_this());
// Continue the recording for the conference if one participant was recording
Expand Down Expand Up @@ -739,14 +732,11 @@ Conference::setActiveParticipant(const std::string& participant_id)
if (!videoMixer_)
return;
if (isHost(participant_id)) {
videoMixer_->addActiveHost();
videoMixer_->setActiveStream(sip_utils::streamId("", 0, MediaType::MEDIA_VIDEO));
return;
}
if (auto call = getCallFromPeerID(participant_id)) {
if (auto videoRecv = call->getReceiveVideoFrameActiveWriter())
videoMixer_->setActiveStream(videoRecv.get());
else
videoMixer_->setActiveStream(call->getCallId());
videoMixer_->setActiveStream(sip_utils::streamId(call->getCallId(), 0, MediaType::MEDIA_VIDEO));
return;
}

Expand All @@ -764,25 +754,13 @@ Conference::setActiveParticipant(const std::string& participant_id)
void
Conference::setActiveStream(const std::string& streamId, bool state)
{
// TODO BUG: for now activeStream is the callId, and should be the sink!
#ifdef ENABLE_VIDEO
if (!videoMixer_)
return;
if (state) {
// TODO remove
if (streamId.empty()) {
videoMixer_->addActiveHost();
} else if (auto call = getCall(streamId)) {
if (auto videoRecv = call->getReceiveVideoFrameActiveWriter())
videoMixer_->setActiveStream(videoRecv.get());
else
videoMixer_->setActiveStream(call->getCallId());
return;
}
// TODO videoMixer_->setActiveStream(sinkId);
} else {
if (state)
videoMixer_->setActiveStream(streamId);
else
videoMixer_->resetActiveStream();
}
#endif
}

Expand Down Expand Up @@ -872,27 +850,6 @@ Conference::createSinks(const ConfInfo& infos)
sink),
confSinksMap_);
}

void
Conference::attachVideo(Observable<std::shared_ptr<MediaFrame>>* frame, const std::string& callId)
{
JAMI_DBG("[conf:%s] attaching video of call %s", id_.c_str(), callId.c_str());
std::lock_guard<std::mutex> lk(videoToCallMtx_);
videoToCall_.emplace(frame, callId);
frame->attach(videoMixer_.get());
}

void
Conference::detachVideo(Observable<std::shared_ptr<MediaFrame>>* frame)
{
std::lock_guard<std::mutex> lk(videoToCallMtx_);
auto it = videoToCall_.find(frame);
if (it != videoToCall_.end()) {
JAMI_DBG("[conf:%s] detaching video of call %s", id_.c_str(), it->second.c_str());
it->first->detach(videoMixer_.get());
videoToCall_.erase(it);
}
}
#endif

void
Expand All @@ -911,8 +868,8 @@ Conference::removeParticipant(const std::string& participant_id)
#ifdef ENABLE_VIDEO
auto sinkId = getConfId() + peerId;
// Remove if active
// TODO if (videoMixer_->verifyActive(sinkId))
if (videoMixer_->verifyActive(participant_id))
// TODO all streams
if (videoMixer_->verifyActive(sip_utils::streamId(participant_id, 0, MediaType::MEDIA_VIDEO)))
videoMixer_->resetActiveStream();
call->exitConference();
if (call->isPeerRecording())
Expand Down Expand Up @@ -1252,7 +1209,6 @@ Conference::setHandRaised(const std::string& deviceId, const bool& state)
if (auto* transport = call->getTransport())
callDeviceId = transport->deviceId();
if (deviceId == callDeviceId) {
JAMI_ERR() << "@@@X";
if (state and not isPeerRequiringAttention) {
JAMI_DBG("Raise %s hand", deviceId.c_str());
handsRaised_.emplace(deviceId);
Expand Down
5 changes: 0 additions & 5 deletions src/conference.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,6 @@ class Conference : public Recordable, public std::enable_shared_from_this<Confer

#ifdef ENABLE_VIDEO
void createSinks(const ConfInfo& infos);
void attachVideo(Observable<std::shared_ptr<MediaFrame>>* frame, const std::string& callId);
void detachVideo(Observable<std::shared_ptr<MediaFrame>>* frame);
std::shared_ptr<video::VideoMixer> getVideoMixer();
std::string getVideoInput() const { return hostVideoSource_.sourceUri_; }
#endif
Expand Down Expand Up @@ -400,9 +398,6 @@ class Conference : public Recordable, public std::enable_shared_from_this<Confer
ConfInfo confInfo_ {};

void sendConferenceInfos();
// We need to convert call to frame
std::mutex videoToCallMtx_;
std::map<Observable<std::shared_ptr<MediaFrame>>*, std::string> videoToCall_ {};
std::shared_ptr<RingBuffer> ghostRingBuffer_;

#ifdef ENABLE_VIDEO
Expand Down
4 changes: 1 addition & 3 deletions src/conference_protocol.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,9 +158,7 @@ ConfProtocolParser::parseV1()
mediaVal[ProtocolKeys::MUTEAUDIO].asBool());
}
if (mediaVal.isMember(ProtocolKeys::ACTIVE)) {
// TODO streamId
setActiveStream_(accountUri,
deviceId,
setActiveStream_(streamId,
mediaVal[ProtocolKeys::ACTIVE].asBool());
}
}
Expand Down
7 changes: 2 additions & 5 deletions src/conference_protocol.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,7 @@ class ConfProtocolParser {
{
raiseHand_ = std::move(cb);
}
/**
* @todo, replace the 2 strings per streamId
*/
void onSetActiveStream(std::function<void(const std::string&, const std::string&, bool)>&& cb)
void onSetActiveStream(std::function<void(const std::string&, bool)>&& cb)
{
setActiveStream_ = std::move(cb);
}
Expand Down Expand Up @@ -125,7 +122,7 @@ class ConfProtocolParser {
std::function<bool(std::string_view)> checkAuthorization_;
std::function<void(const std::string&, const std::string&)> hangupParticipant_;
std::function<void(const std::string&, bool)> raiseHand_;
std::function<void(const std::string&, const std::string&, bool)> setActiveStream_;
std::function<void(const std::string&, bool)> setActiveStream_;
std::function<void(const std::string&, const std::string&, const std::string&, bool)>
muteStreamAudio_;
std::function<void(const std::string&, const std::string&, const std::string&, bool)>
Expand Down
20 changes: 10 additions & 10 deletions src/media/audio/audio_rtp_session.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,21 @@

namespace jami {

AudioRtpSession::AudioRtpSession(const std::string& id)
: RtpSession(id, MediaType::MEDIA_AUDIO)
AudioRtpSession::AudioRtpSession(const std::string& callId, const std::string& streamId)
: RtpSession(callId, streamId, MediaType::MEDIA_AUDIO)
, rtcpCheckerThread_([] { return true; }, [this] { processRtcpChecker(); }, [] {})

{
JAMI_DBG("Created Audio RTP session: %p - call Id %s", this, callID_.c_str());
JAMI_DBG("Created Audio RTP session: %p - call Id %s", this, callId_.c_str());

// don't move this into the initializer list or Cthulus will emerge
ringbuffer_ = Manager::instance().getRingBufferPool().createRingBuffer(callID_);
ringbuffer_ = Manager::instance().getRingBufferPool().createRingBuffer(callId_);
}

AudioRtpSession::~AudioRtpSession()
{
stop();
JAMI_DBG("Destroyed Audio RTP session: %p - call Id %s", this, callID_.c_str());
JAMI_DBG("Destroyed Audio RTP session: %p - call Id %s", this, callId_.c_str());
}

void
Expand Down Expand Up @@ -90,7 +90,7 @@ AudioRtpSession::startSender()
audioInput_->detach(sender_.get());

// sender sets up input correctly, we just keep a reference in case startSender is called
audioInput_ = jami::getAudioInput(callID_);
audioInput_ = jami::getAudioInput(callId_);
audioInput_->setMuted(muteState_);
audioInput_->setSuccessfulSetupCb(onSuccessfulSetup_);
auto newParams = audioInput_->switchInput(input_);
Expand All @@ -117,7 +117,7 @@ AudioRtpSession::startSender()
sender_.reset();
socketPair_->stopSendOp(false);
sender_.reset(
new AudioSender(callID_, getRemoteRtpUri(), send_, *socketPair_, initSeqVal_, mtu_));
new AudioSender(getRemoteRtpUri(), send_, *socketPair_, initSeqVal_, mtu_));
} catch (const MediaEncoderException& e) {
JAMI_ERR("%s", e.what());
send_.enabled = false;
Expand Down Expand Up @@ -160,7 +160,7 @@ AudioRtpSession::startReceiver()
JAMI_WARN("Restarting audio receiver");

auto accountAudioCodec = std::static_pointer_cast<AccountAudioCodecInfo>(receive_.codec);
receiveThread_.reset(new AudioReceiveThread(callID_,
receiveThread_.reset(new AudioReceiveThread(callId_,
accountAudioCodec->audioformat,
receive_.receiving_sdp,
mtu_));
Expand Down Expand Up @@ -332,7 +332,7 @@ AudioRtpSession::initRecorder(std::shared_ptr<MediaRecorder>& rec)
{
if (receiveThread_)
receiveThread_->attach(rec->addStream(receiveThread_->getInfo()));
if (auto input = jami::getAudioInput(callID_))
if (auto input = jami::getAudioInput(callId_))
input->attach(rec->addStream(input->getInfo()));
}

Expand All @@ -344,7 +344,7 @@ AudioRtpSession::deinitRecorder(std::shared_ptr<MediaRecorder>& rec)
receiveThread_->detach(ob);
}
}
if (auto input = jami::getAudioInput(callID_)) {
if (auto input = jami::getAudioInput(callId_)) {
if (auto ob = rec->getStream(input->getInfo().name)) {
input->detach(ob);
}
Expand Down
2 changes: 1 addition & 1 deletion src/media/audio/audio_rtp_session.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ struct RTCPInfo
class AudioRtpSession : public RtpSession
{
public:
AudioRtpSession(const std::string& id);
AudioRtpSession(const std::string& callId, const std::string& streamId);
virtual ~AudioRtpSession();

void start(std::unique_ptr<IceSocket> rtp_sock, std::unique_ptr<IceSocket> rtcp_sock) override;
Expand Down
Loading

0 comments on commit ce645d4

Please sign in to comment.