Skip to content

Commit

Permalink
Intermittent GST_QUERY_POSITION failure on XiOne UK (#86)
Browse files Browse the repository at this point in the history 
Summary: Fixed intermittent GST_QUERY_POSITION failure on XiOne UK
Type: Feature
Test Plan: Unittests/Fullstack tests
Jira: RIALTO-446
  • Loading branch information
@aczs
aczs committed Jan 4, 2024
1 parent 0efb918 commit 16ef7a0
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 52 deletions.
40 changes: 28 additions & 12 deletions source/GStreamerMSEMediaPlayerClient.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ GStreamerMSEMediaPlayerClient::GStreamerMSEMediaPlayerClient(
const std::shared_ptr<firebolt::rialto::client::MediaPlayerClientBackendInterface> &MediaPlayerClientBackend,
const uint32_t maxVideoWidth, const uint32_t maxVideoHeight)
: m_backendQueue{messageQueueFactory->createMessageQueue()}, m_messageQueueFactory{messageQueueFactory},
m_clientBackend(MediaPlayerClientBackend), m_position(0), m_duration(0), m_audioStreams{UNKNOWN_STREAMS_NUMBER},
m_clientBackend(MediaPlayerClientBackend), m_duration(0), m_audioStreams{UNKNOWN_STREAMS_NUMBER},
m_videoStreams{UNKNOWN_STREAMS_NUMBER}, m_videoRectangle{0, 0, 1920, 1080}, m_streamingStopped(false),
m_maxWidth(maxVideoWidth == 0 ? DEFAULT_MAX_VIDEO_WIDTH : maxVideoWidth),
m_maxHeight(maxVideoHeight == 0 ? DEFAULT_MAX_VIDEO_HEIGHT : maxVideoHeight)
Expand Down Expand Up @@ -79,7 +79,7 @@ void GStreamerMSEMediaPlayerClient::notifyDuration(int64_t duration)

void GStreamerMSEMediaPlayerClient::notifyPosition(int64_t position)
{
m_backendQueue->postMessage(std::make_shared<SetPositionMessage>(position, m_position));
m_backendQueue->postMessage(std::make_shared<SetPositionMessage>(position, m_attachedSources));
}

void GStreamerMSEMediaPlayerClient::notifyNativeSize(uint32_t width, uint32_t height, double aspect) {}
Expand Down Expand Up @@ -116,22 +116,29 @@ void GStreamerMSEMediaPlayerClient::notifyBufferUnderflow(int32_t sourceId)
m_backendQueue->postMessage(std::make_shared<BufferUnderflowMessage>(sourceId, this));
}

void GStreamerMSEMediaPlayerClient::getPositionDo(int64_t *position)
void GStreamerMSEMediaPlayerClient::getPositionDo(int64_t *position, int32_t sourceId)
{
auto sourceIt = m_attachedSources.find(sourceId);
if (sourceIt == m_attachedSources.end())
{
*position = -1;
return;
}

if (m_clientBackend && m_clientBackend->getPosition(*position))
{
m_position = *position;
sourceIt->second.m_position = *position;
}
else
{
*position = m_position;
*position = sourceIt->second.m_position;
}
}

int64_t GStreamerMSEMediaPlayerClient::getPosition()
int64_t GStreamerMSEMediaPlayerClient::getPosition(int32_t sourceId)
{
int64_t position;
m_backendQueue->callInEventLoop([&]() { getPositionDo(&position); });
m_backendQueue->callInEventLoop([&]() { getPositionDo(&position, sourceId); });
return position;
}

Expand Down Expand Up @@ -209,7 +216,10 @@ void GStreamerMSEMediaPlayerClient::seek(int64_t seekPosition)
{
m_serverSeekingState = SeekingState::SEEKING;
m_clientBackend->seek(seekPosition);
m_position = seekPosition;
for (auto &source : m_attachedSources)
{
source.second.m_position = seekPosition;
}
});
}

Expand Down Expand Up @@ -366,7 +376,10 @@ void GStreamerMSEMediaPlayerClient::handlePlaybackStateChange(firebolt::rialto::
rialto_mse_base_handle_rialto_server_error(source.second.m_rialtoSink);
}
m_serverSeekingState = SeekingState::IDLE;
m_position = 0;
for (auto &source : m_attachedSources)
{
source.second.m_position = 0;
}

break;
}
Expand Down Expand Up @@ -780,14 +793,17 @@ void BufferUnderflowMessage::handle()
}
}

SetPositionMessage::SetPositionMessage(int64_t newPosition, int64_t &targetPosition)
: m_newPosition(newPosition), m_targetPosition(targetPosition)
SetPositionMessage::SetPositionMessage(int64_t newPosition, std::unordered_map<int32_t, AttachedSource> &attachedSources)
: m_newPosition(newPosition), m_attachedSources(attachedSources)
{
}

void SetPositionMessage::handle()
{
m_targetPosition = m_newPosition;
for (auto &source : m_attachedSources)
{
source.second.setPosition(m_newPosition);
}
}

SetDurationMessage::SetDurationMessage(int64_t newDuration, int64_t &targetDuration)
Expand Down
66 changes: 34 additions & 32 deletions source/GStreamerMSEMediaPlayerClient.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,13 @@

class GStreamerMSEMediaPlayerClient;

enum class SeekingState
{
IDLE,
SEEKING,
SEEK_DONE
};

class BufferPuller
{
public:
Expand All @@ -64,6 +71,29 @@ class BufferPuller
std::shared_ptr<BufferParser> m_bufferParser;
};

class AttachedSource
{
friend class GStreamerMSEMediaPlayerClient;

public:
AttachedSource(RialtoMSEBaseSink *rialtoSink, std::shared_ptr<BufferPuller> puller,
firebolt::rialto::MediaSourceType type)
: m_rialtoSink(rialtoSink), m_bufferPuller(puller), m_type(type)
{
}

firebolt::rialto::MediaSourceType getType() { return m_type; }
void setPosition(int64_t position) { m_position = position; }

private:
RialtoMSEBaseSink *m_rialtoSink;
std::shared_ptr<BufferPuller> m_bufferPuller;
SeekingState m_seekingState = SeekingState::IDLE;
std::unordered_set<uint32_t> m_ongoingNeedDataRequests;
firebolt::rialto::MediaSourceType m_type = firebolt::rialto::MediaSourceType::UNKNOWN;
int64_t m_position = 0;
};

class HaveDataMessage : public Message
{
public:
Expand Down Expand Up @@ -147,12 +177,12 @@ class BufferUnderflowMessage : public Message
class SetPositionMessage : public Message
{
public:
SetPositionMessage(int64_t newPosition, int64_t &targetPosition);
SetPositionMessage(int64_t newPosition, std::unordered_map<int32_t, AttachedSource> &attachedSources);
void handle() override;

private:
int64_t m_newPosition;
int64_t &m_targetPosition;
std::unordered_map<int32_t, AttachedSource> &m_attachedSources;
};

class SetDurationMessage : public Message
Expand All @@ -166,34 +196,6 @@ class SetDurationMessage : public Message
int64_t &m_targetDuration;
};

enum class SeekingState
{
IDLE,
SEEKING,
SEEK_DONE
};

class AttachedSource
{
friend class GStreamerMSEMediaPlayerClient;

public:
AttachedSource(RialtoMSEBaseSink *rialtoSink, std::shared_ptr<BufferPuller> puller,
firebolt::rialto::MediaSourceType type)
: m_rialtoSink(rialtoSink), m_bufferPuller(puller), m_type(type)
{
}

firebolt::rialto::MediaSourceType getType() { return m_type; }

private:
RialtoMSEBaseSink *m_rialtoSink;
std::shared_ptr<BufferPuller> m_bufferPuller;
SeekingState m_seekingState = SeekingState::IDLE;
std::unordered_set<uint32_t> m_ongoingNeedDataRequests;
firebolt::rialto::MediaSourceType m_type = firebolt::rialto::MediaSourceType::UNKNOWN;
};

class GStreamerMSEMediaPlayerClient : public firebolt::rialto::IMediaPipelineClient,
public std::enable_shared_from_this<GStreamerMSEMediaPlayerClient>
{
Expand Down Expand Up @@ -222,8 +224,8 @@ class GStreamerMSEMediaPlayerClient : public firebolt::rialto::IMediaPipelineCli
void notifyQos(int32_t sourceId, const firebolt::rialto::QosInfo &qosInfo) override;
void notifyBufferUnderflow(int32_t sourceId) override;

void getPositionDo(int64_t *position);
int64_t getPosition();
void getPositionDo(int64_t *position, int32_t sourceId);
int64_t getPosition(int32_t sourceId);
firebolt::rialto::AddSegmentStatus
addSegment(unsigned int needDataRequestId,
const std::unique_ptr<firebolt::rialto::IMediaPipeline::MediaSegment> &mediaSegment);
Expand Down
4 changes: 2 additions & 2 deletions source/RialtoGStreamerMSEBaseSink.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ static gboolean rialto_mse_base_sink_query(GstElement *element, GstQuery *query)
case GST_QUERY_POSITION:
{
std::shared_ptr<GStreamerMSEMediaPlayerClient> client = sink->priv->m_mediaPlayerManager.getMediaPlayerClient();
if ((!client) || (!sink->priv->m_mediaPlayerManager.hasControl()))
if (!client)
{
return FALSE;
}
Expand All @@ -263,7 +263,7 @@ static gboolean rialto_mse_base_sink_query(GstElement *element, GstQuery *query)
{
case GST_FORMAT_TIME:
{
gint64 position = client->getPosition();
gint64 position = client->getPosition(sink->priv->m_sourceId);
GST_DEBUG_OBJECT(sink, "Queried position is %" GST_TIME_FORMAT, GST_TIME_ARGS(position));
if (position < 0)
{
Expand Down
27 changes: 25 additions & 2 deletions tests/ut/GstreamerMseBaseSinkTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -268,19 +268,38 @@ TEST_F(GstreamerMseBaseSinkTests, ShouldFailToQueryPositionWhenPipelineIsBelowPa
gst_object_unref(audioSink);
}

TEST_F(GstreamerMseBaseSinkTests, ShouldFailToQueryPositionWhenSourceNotAttached)
{
RialtoMSEBaseSink *audioSink = createAudioSink();
GstElement *pipeline = createPipelineWithSink(audioSink);

setPausedState(pipeline, audioSink);

gint64 position{0};
EXPECT_FALSE(gst_element_query_position(GST_ELEMENT_CAST(audioSink), GST_FORMAT_TIME, &position));

setNullState(pipeline, kUnknownSourceId);

gst_object_unref(pipeline);
}

TEST_F(GstreamerMseBaseSinkTests, ShouldFailToQueryPositionWhenPositionIsInvalid)
{
constexpr gint64 kInvalidPosition{-1};
RialtoMSEBaseSink *audioSink = createAudioSink();
GstElement *pipeline = createPipelineWithSink(audioSink);

setPausedState(pipeline, audioSink);
const int32_t kSourceId{audioSourceWillBeAttached(createAudioMediaSource())};
GstCaps *caps{createAudioCaps()};
setCaps(audioSink, caps);

gint64 position{0};
EXPECT_CALL(m_mediaPipelineMock, getPosition(_)).WillOnce(DoAll(SetArgReferee<0>(kInvalidPosition), Return(true)));
EXPECT_FALSE(gst_element_query_position(GST_ELEMENT_CAST(audioSink), GST_FORMAT_TIME, &position));

setNullState(pipeline, kUnknownSourceId);
setNullState(pipeline, kSourceId);
gst_caps_unref(caps);
gst_object_unref(pipeline);
}

Expand All @@ -291,13 +310,17 @@ TEST_F(GstreamerMseBaseSinkTests, ShouldQueryPosition)
GstElement *pipeline = createPipelineWithSink(audioSink);

setPausedState(pipeline, audioSink);
const int32_t kSourceId{audioSourceWillBeAttached(createAudioMediaSource())};
GstCaps *caps{createAudioCaps()};
setCaps(audioSink, caps);

gint64 position{0};
EXPECT_CALL(m_mediaPipelineMock, getPosition(_)).WillOnce(DoAll(SetArgReferee<0>(kPosition), Return(true)));
EXPECT_TRUE(gst_element_query_position(GST_ELEMENT_CAST(audioSink), GST_FORMAT_TIME, &position));
EXPECT_EQ(position, kPosition);

setNullState(pipeline, kUnknownSourceId);
setNullState(pipeline, kSourceId);
gst_caps_unref(caps);
gst_object_unref(pipeline);
}

Expand Down
25 changes: 21 additions & 4 deletions tests/ut/GstreamerMseMediaPlayerClientTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,16 @@ TEST_F(GstreamerMseMediaPlayerClientTests, ShouldNotifyDuration)

TEST_F(GstreamerMseMediaPlayerClientTests, ShouldNotifyPosition)
{
RialtoMSEBaseSink *audioSink = createAudioSink();
bufferPullerWillBeCreated();
const int32_t kSourceId{attachSource(audioSink, firebolt::rialto::MediaSourceType::AUDIO)};
expectPostMessage();
expectCallInEventLoop();
m_sut->notifyPosition(kPosition);
m_sut->destroyClientBackend();
EXPECT_EQ(m_sut->getPosition(), kPosition);
EXPECT_EQ(m_sut->getPosition(kSourceId), kPosition);

gst_object_unref(audioSink);
}

TEST_F(GstreamerMseMediaPlayerClientTests, ShouldNotifyNativeSize)
Expand Down Expand Up @@ -243,12 +248,17 @@ TEST_F(GstreamerMseMediaPlayerClientTests, ShouldReceiveUnexpectedFlushedMessage

TEST_F(GstreamerMseMediaPlayerClientTests, ShouldReceiveFailureMessage)
{
RialtoMSEBaseSink *audioSink = createAudioSink();
bufferPullerWillBeCreated();
const int32_t kSourceId{attachSource(audioSink, firebolt::rialto::MediaSourceType::AUDIO)};
expectPostMessage();
expectCallInEventLoop();
m_sut->notifyPlaybackState(firebolt::rialto::PlaybackState::FAILURE);
// Position should be set to 0
EXPECT_CALL(*m_mediaPlayerClientBackendMock, getPosition(_)).WillOnce(Return(false));
EXPECT_EQ(m_sut->getPosition(), 0);
EXPECT_EQ(m_sut->getPosition(kSourceId), 0);

gst_object_unref(audioSink);
}

TEST_F(GstreamerMseMediaPlayerClientTests, ShouldNotifyVideoData)
Expand Down Expand Up @@ -465,9 +475,14 @@ TEST_F(GstreamerMseMediaPlayerClientTests, ShouldFailToNotifyBufferUnderflowWhen

TEST_F(GstreamerMseMediaPlayerClientTests, ShouldGetPosition)
{
RialtoMSEBaseSink *audioSink = createAudioSink();
bufferPullerWillBeCreated();
const int32_t kSourceId{attachSource(audioSink, firebolt::rialto::MediaSourceType::AUDIO)};
EXPECT_CALL(*m_mediaPlayerClientBackendMock, getPosition(_)).WillOnce(DoAll(SetArgReferee<0>(kPosition), Return(true)));
expectCallInEventLoop();
EXPECT_EQ(m_sut->getPosition(), kPosition);
EXPECT_EQ(m_sut->getPosition(kSourceId), kPosition);

gst_object_unref(audioSink);
}

TEST_F(GstreamerMseMediaPlayerClientTests, ShouldFailToCreateBackend)
Expand Down Expand Up @@ -576,6 +591,8 @@ TEST_F(GstreamerMseMediaPlayerClientTests, ShouldFinishSeekWhenFailureStateIsRec
{
RialtoMSEBaseSink *audioSink = createAudioSink();
bufferPullerWillBeCreated();
const int32_t kSourceId{attachSource(audioSink, firebolt::rialto::MediaSourceType::AUDIO)};
bufferPullerWillBeCreated();
attachSource(audioSink, firebolt::rialto::MediaSourceType::AUDIO);
EXPECT_CALL(*m_mediaPlayerClientBackendMock, seek(kPosition)).WillOnce(Return(true));
m_sut->seek(kPosition);
Expand All @@ -585,7 +602,7 @@ TEST_F(GstreamerMseMediaPlayerClientTests, ShouldFinishSeekWhenFailureStateIsRec

// Position should be reset to 0
EXPECT_CALL(*m_mediaPlayerClientBackendMock, getPosition(_)).WillOnce(Return(false));
EXPECT_EQ(m_sut->getPosition(), 0);
EXPECT_EQ(m_sut->getPosition(kSourceId), 0);

gst_object_unref(audioSink);
}
Expand Down

0 comments on commit 16ef7a0

Please sign in to comment.