Bug 1305949 - Refactor code that feeds video stream sink when it gets added. r?ctai
This mostly simplifies the code, but there are two changes to the logic as well.
1) The decision to install the listener or not used to be based on if the track
existed in mUpdateTracks, while feeding the sink would look at the
StreamTracks as well. This now looks at StreamTracks since an ended track is
kept there but removed from mUpdateTracks. That means that we also
NotifyEnded() if the track was in the StreamTracks but not in mUpdateTracks.
2) I removed the code that feeds a video stream sink with the last chunk in
case the graph's current time has passed the end of the track. Tests should
be written so that we have guarantees that there will be video data when a
video stream sink gets added. If this fails we should fix the root cause of
such a timing issue instead of wallpapering it with an old frame. I think
this could be masking other issues. We'll see how this acts out on try.
MozReview-Commit-ID: KKr9JhVpxZt
--- a/dom/media/MediaStreamGraph.cpp
+++ b/dom/media/MediaStreamGraph.cpp
@@ -2860,96 +2860,88 @@ SourceMediaStream::RemoveDirectListener(
}
}
void
SourceMediaStream::AddDirectTrackListenerImpl(already_AddRefed<DirectMediaStreamTrackListener> aListener,
TrackID aTrackID)
{
MOZ_ASSERT(IsTrackIDExplicit(aTrackID));
- TrackData* data;
- bool found = false;
+ TrackData* updateData = nullptr;
+ StreamTracks::Track* track = nullptr;
bool isAudio = false;
bool isVideo = false;
RefPtr<DirectMediaStreamTrackListener> listener = aListener;
STREAM_LOG(LogLevel::Debug, ("Adding direct track listener %p bound to track %d to source stream %p",
listener.get(), aTrackID, this));
{
MutexAutoLock lock(mMutex);
- data = FindDataForTrack(aTrackID);
- found = !!data;
- if (found) {
- isAudio = data->mData->GetType() == MediaSegment::AUDIO;
- isVideo = data->mData->GetType() == MediaSegment::VIDEO;
+ updateData = FindDataForTrack(aTrackID);
+ track = FindTrack(aTrackID);
+ if (track) {
+ isAudio = track->GetType() == MediaSegment::AUDIO;
+ isVideo = track->GetType() == MediaSegment::VIDEO;
}
- // The track might be removed from mUpdateTrack but still exist in
- // mTracks.
- auto streamTrack = FindTrack(aTrackID);
- bool foundTrack = !!streamTrack;
- if (foundTrack) {
- MediaStreamVideoSink* videoSink = listener->AsMediaStreamVideoSink();
+ MediaStreamVideoSink* videoSink = listener->AsMediaStreamVideoSink();
+ if (track && isVideo && videoSink) {
// Re-send missed VideoSegment to new added MediaStreamVideoSink.
- if (streamTrack->GetType() == MediaSegment::VIDEO && videoSink) {
- VideoSegment videoSegment;
- if (mTracks.GetForgottenDuration() < streamTrack->GetSegment()->GetDuration()) {
- videoSegment.AppendSlice(*streamTrack->GetSegment(),
- mTracks.GetForgottenDuration(),
- streamTrack->GetSegment()->GetDuration());
- } else {
- VideoSegment* streamTrackSegment = static_cast<VideoSegment*>(streamTrack->GetSegment());
- VideoChunk* lastChunk = streamTrackSegment->GetLastChunk();
- if (lastChunk) {
- StreamTime startTime = streamTrackSegment->GetDuration() - lastChunk->GetDuration();
- videoSegment.AppendSlice(*streamTrackSegment,
- startTime,
- streamTrackSegment->GetDuration());
- }
- }
- if (found) {
- videoSegment.AppendSlice(*data->mData, 0, data->mData->GetDuration());
- }
- videoSink->SetCurrentFrames(videoSegment);
+ VideoSegment* trackSegment = static_cast<VideoSegment*>(track->GetSegment());
+ VideoSegment videoSegment;
+ if (mTracks.GetForgottenDuration() < trackSegment->GetDuration()) {
+ videoSegment.AppendSlice(*trackSegment,
+ mTracks.GetForgottenDuration(),
+ trackSegment->GetDuration());
}
+ if (updateData) {
+ videoSegment.AppendSlice(*updateData->mData, 0, updateData->mData->GetDuration());
+ }
+ videoSink->SetCurrentFrames(videoSegment);
}
- if (found && (isAudio || isVideo)) {
+ if (track && (isAudio || isVideo)) {
for (auto entry : mDirectTrackListeners) {
if (entry.mListener == listener &&
(entry.mTrackID == TRACK_ANY || entry.mTrackID == aTrackID)) {
listener->NotifyDirectListenerInstalled(
DirectMediaStreamTrackListener::InstallationResult::ALREADY_EXISTS);
return;
}
}
TrackBound<DirectMediaStreamTrackListener>* sourceListener =
mDirectTrackListeners.AppendElement();
sourceListener->mListener = listener;
sourceListener->mTrackID = aTrackID;
}
}
- if (!found) {
+ if (!track) {
STREAM_LOG(LogLevel::Warning, ("Couldn't find source track for direct track listener %p",
listener.get()));
listener->NotifyDirectListenerInstalled(
DirectMediaStreamTrackListener::InstallationResult::TRACK_NOT_FOUND_AT_SOURCE);
return;
}
if (!isAudio && !isVideo) {
STREAM_LOG(LogLevel::Warning, ("Source track for direct track listener %p is unknown",
listener.get()));
// It is not a video or audio track.
MOZ_ASSERT(true);
return;
}
- STREAM_LOG(LogLevel::Debug, ("Added direct track listener %p", listener.get()));
+ STREAM_LOG(LogLevel::Debug, ("Added direct track listener %p. ended=%d",
+ listener.get(), !updateData));
listener->NotifyDirectListenerInstalled(
DirectMediaStreamTrackListener::InstallationResult::SUCCESS);
+ if (!updateData) {
+ // The track exists but the mUpdateTracks entry was removed.
+ // This means that the track has ended.
+ listener->NotifyEnded();
+ }
}
void
SourceMediaStream::RemoveDirectTrackListenerImpl(DirectMediaStreamTrackListener* aListener,
TrackID aTrackID)
{
MutexAutoLock lock(mMutex);
for (int32_t i = mDirectTrackListeners.Length() - 1; i >= 0; --i) {