-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid using publisher from OTM #1556
Avoid using publisher from OTM #1556
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 👍
@@ -69,13 +70,14 @@ namespace erizo { | |||
return 0; | |||
} | |||
boost::unique_lock<boost::mutex> lock(monitor_mutex_); | |||
if (subscribers.empty()) | |||
if (subscribers_.empty() || !publisher_) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
erizo/src/erizo/OneToManyProcessor.h
Outdated
int deliverAudioData_(std::shared_ptr<DataPacket> audio_packet) override; | ||
int deliverVideoData_(std::shared_ptr<DataPacket> video_packet) override; | ||
int deliverFeedback_(std::shared_ptr<DataPacket> fb_packet) override; | ||
int deliverEvent_(MediaEventPtr event) override; | ||
boost::future<void> closeAll(); | ||
bool isSSRCFromAudio(uint32_t ssrc); | ||
uint32_t translateAndMaybeAdaptForSimulcast(uint32_t orig_ssrc); | ||
|
||
private: | ||
typedef std::shared_ptr<MediaSink> sink_ptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💯 to the variable names updates. In that line, shouldn't this be sink_ptr_
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually we can remove it because we don't use it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a typedef instead, but we don't use to define any variable in this class
@@ -165,7 +165,7 @@ NAN_METHOD(OneToManyProcessor::getPublisherState) { | |||
return; | |||
} | |||
|
|||
auto wr = std::dynamic_pointer_cast<erizo::MediaStream>(me->publisher); | |||
auto wr = std::dynamic_pointer_cast<erizo::MediaStream>(me->getPublisher()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Description
There have been other crashes inside OneToManyProcessor when trying to access publisher's stuff.
[] It needs and includes Unit Tests
Changes in Client or Server public APIs
Not needed.
[] It includes documentation for these changes in
/doc
.