Merge lp:~phablet-team/qtubuntu-media/fix-1506953 into lp:qtubuntu-media

Proposed by Jim Hodapp
Status: Merged
Approved by: Konrad Zapałowicz
Approved revision: 114
Merged at revision: 111
Proposed branch: lp:~phablet-team/qtubuntu-media/fix-1506953
Merge into: lp:qtubuntu-media
Diff against target: 438 lines (+164/-34)
8 files modified
debian/control (+1/-1)
src/aal/aalmediaplayerservice.cpp (+79/-8)
src/aal/aalmediaplayerservice.h (+4/-1)
src/aal/aalmediaplaylistcontrol.cpp (+55/-19)
src/aal/aalmediaplaylistprovider.cpp (+2/-4)
src/aal/aalvideorenderercontrol.cpp (+8/-1)
tests/unit/service.cpp (+12/-0)
tests/unit/service.h (+3/-0)
To merge this branch: bzr merge lp:~phablet-team/qtubuntu-media/fix-1506953
Reviewer Review Type Date Requested Status
Konrad Zapałowicz (community) code Approve
Review via email: mp+300533@code.launchpad.net

Commit message

Register for media-hub service unregister/register signals and raise an error to the client application when media-hub-server unregisters and then re-registers on dbus. This will allow the client to handle this event as it deems appropriate.

Description of the change

Register for media-hub service unregister/register signals and raise an error to the client application when media-hub-server unregisters and then re-registers on dbus. This will allow the client to handle this event as it deems appropriate.

To post a comment you must log in.
112. By Jim Hodapp

Merge with upstream

113. By Jim Hodapp

Clean up unnecessary debug statements

114. By Jim Hodapp

Another unnessary debug statement

Revision history for this message
Konrad Zapałowicz (kzapalowicz) wrote :

LGTM

review: Approve (code)
115. By Jim Hodapp

Some additional protection from unhandled exceptions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'debian/control'
2--- debian/control 2016-07-11 01:04:02 +0000
3+++ debian/control 2016-07-20 21:27:06 +0000
4@@ -9,7 +9,7 @@
5 libgl1-mesa-dev | libgl-dev,
6 libgles2-mesa-dev,
7 libhybris-dev (>= 0.1.0+git20131207+e452e83-0ubuntu13),
8- libmedia-hub-dev (>= 4.4.0),
9+ libmedia-hub-dev (>= 4.5.0),
10 libpulse-dev,
11 libqt5opengl5-dev,
12 libqtubuntu-media-signals-dev (>= 0.3+15.04.20150618.1-0ubuntu1),
13
14=== modified file 'src/aal/aalmediaplayerservice.cpp'
15--- src/aal/aalmediaplayerservice.cpp 2016-06-09 18:34:21 +0000
16+++ src/aal/aalmediaplayerservice.cpp 2016-07-20 21:27:06 +0000
17@@ -63,13 +63,42 @@
18 core::Signal<void> the_void;
19 }
20
21+class EmptySink : public core::ubuntu::media::video::Sink
22+{
23+public:
24+ EmptySink(std::uint32_t gl_texture)
25+ {
26+ Q_UNUSED(gl_texture);
27+ }
28+
29+ const core::Signal<void>& frame_available() const override
30+ {
31+ static const core::Signal<void> frame_available;
32+ return frame_available;
33+ }
34+
35+ bool transformation_matrix(float* matrix) const override
36+ {
37+ Q_UNUSED(matrix);
38+ return false;
39+ }
40+
41+ bool swap_buffers() const override
42+ {
43+ return false;
44+ }
45+};
46+
47+
48 AalMediaPlayerService::AalMediaPlayerService(QObject *parent)
49 :
50 QMediaService(parent),
51- m_hubPlayerSession(NULL),
52+ m_hubPlayerSession(nullptr),
53 m_playbackStatusChangedConnection(the_void.connect([](){})),
54 m_errorConnection(the_void.connect([](){})),
55 m_endOfStreamConnection(the_void.connect([](){})),
56+ m_serviceDisconnectedConnection(the_void.connect([](){})),
57+ m_serviceReconnectedConnection(the_void.connect([](){})),
58 m_bufferingStatusChangedConnection(the_void.connect([](){})),
59 m_mediaPlayerControl(nullptr),
60 m_videoOutput(nullptr),
61@@ -79,7 +108,7 @@
62 m_videoOutputReady(false),
63 m_firstPlayback(true),
64 m_cachedDuration(0),
65- m_mediaPlaylist(NULL),
66+ m_mediaPlaylist(nullptr),
67 m_bufferPercent(0),
68 m_doReattachSession(false)
69 #ifdef MEASURE_PERFORMANCE
70@@ -100,10 +129,12 @@
71 :
72 QMediaService(parent),
73 m_hubService(service),
74- m_hubPlayerSession(NULL),
75+ m_hubPlayerSession(nullptr),
76 m_playbackStatusChangedConnection(the_void.connect([](){})),
77 m_errorConnection(the_void.connect([](){})),
78 m_endOfStreamConnection(the_void.connect([](){})),
79+ m_serviceDisconnectedConnection(the_void.connect([](){})),
80+ m_serviceReconnectedConnection(the_void.connect([](){})),
81 m_bufferingStatusChangedConnection(the_void.connect([](){})),
82 m_mediaPlayerControl(nullptr),
83 m_videoOutput(nullptr),
84@@ -113,7 +144,7 @@
85 m_videoOutputReady(false),
86 m_firstPlayback(true),
87 m_cachedDuration(0),
88- m_mediaPlaylist(NULL),
89+ m_mediaPlaylist(nullptr),
90 m_bufferPercent(0),
91 m_doReattachSession(false)
92 #ifdef MEASURE_PERFORMANCE
93@@ -133,6 +164,8 @@
94 {
95 m_errorConnection.disconnect();
96 m_playbackStatusChangedConnection.disconnect();
97+ m_serviceDisconnectedConnection.disconnect();
98+ m_serviceReconnectedConnection.disconnect();
99
100 if (m_audioRoleControl)
101 deleteAudioRoleControl();
102@@ -160,11 +193,13 @@
103 // it again. If we don't do this connectSignals() will never be able to attach
104 // to the relevant signals.
105 m_endOfStreamConnection.disconnect();
106+ m_serviceDisconnectedConnection.disconnect();
107+ m_serviceReconnectedConnection.disconnect();
108
109 if (!newMediaPlayer())
110 qWarning() << "Failed to create a new media player backend. Video playback will not function." << endl;
111
112- if (m_hubPlayerSession == NULL)
113+ if (m_hubPlayerSession == nullptr)
114 {
115 qWarning() << "Could not finish contructing new AalMediaPlayerService instance since m_hubPlayerSession is NULL";
116 return;
117@@ -262,7 +297,7 @@
118 // changes
119 m_sessionUuid = m_hubPlayerSession->uuid();
120 } catch (const std::runtime_error &e) {
121- qWarning() << "Failed to retrieve the current player's uuid: " << e.what() << endl;
122+ qWarning() << "Failed to retrieve the current player's uuid: " << e.what();
123 return false;
124 }
125
126@@ -271,13 +306,19 @@
127
128 std::shared_ptr<core::ubuntu::media::video::Sink> AalMediaPlayerService::createVideoSink(uint32_t texture_id)
129 {
130- if (m_hubPlayerSession == NULL)
131+ if (m_hubPlayerSession == nullptr)
132 throw std::runtime_error
133 {
134 "Cannot create a video sink without a valid media-hub player session"
135 };
136
137- auto sink = m_hubPlayerSession->create_gl_texture_video_sink(texture_id);
138+ std::shared_ptr<core::ubuntu::media::video::Sink> sink;
139+ try {
140+ sink = m_hubPlayerSession->create_gl_texture_video_sink(texture_id);
141+ } catch (const std::runtime_error &e) {
142+ qWarning() << "Failed to create a new video sink:" << e.what();
143+ return core::ubuntu::media::video::Sink::Ptr{new EmptySink(0)};
144+ }
145 m_videoOutputReady = true;
146
147 return sink;
148@@ -782,6 +823,20 @@
149 }
150 }
151
152+void AalMediaPlayerService::onServiceDisconnected()
153+{
154+ qDebug() << Q_FUNC_INFO;
155+ m_mediaPlayerControl->setState(QMediaPlayer::StoppedState);
156+ m_mediaPlayerControl->setMediaStatus(QMediaPlayer::NoMedia);
157+}
158+
159+void AalMediaPlayerService::onServiceReconnected()
160+{
161+ qDebug() << Q_FUNC_INFO;
162+ const QString errStr = "Player session is no longer valid since the service restarted.";
163+ m_mediaPlayerControl->error(QMediaPlayer::ServiceMissingError, errStr);
164+}
165+
166 void AalMediaPlayerService::onBufferingChanged()
167 {
168 Q_EMIT m_mediaPlayerControl->bufferStatusChanged(m_bufferPercent);
169@@ -822,6 +877,22 @@
170 Q_EMIT playbackComplete();
171 });
172 }
173+
174+ if (!m_serviceDisconnectedConnection.is_connected())
175+ {
176+ m_serviceDisconnectedConnection = m_hubService->service_disconnected().connect([this]()
177+ {
178+ QMetaObject::invokeMethod(this, "onServiceDisconnected", Qt::QueuedConnection);
179+ });
180+ }
181+
182+ if (!m_serviceReconnectedConnection.is_connected())
183+ {
184+ m_serviceReconnectedConnection = m_hubService->service_reconnected().connect([this]()
185+ {
186+ QMetaObject::invokeMethod(this, "onServiceReconnected", Qt::QueuedConnection);
187+ });
188+ }
189 }
190
191 void AalMediaPlayerService::disconnectSignals()
192
193=== modified file 'src/aal/aalmediaplayerservice.h'
194--- src/aal/aalmediaplayerservice.h 2016-06-08 09:17:31 +0000
195+++ src/aal/aalmediaplayerservice.h 2016-07-20 21:27:06 +0000
196@@ -113,6 +113,8 @@
197 public Q_SLOTS:
198 void onPlaybackStatusChanged();
199 void onApplicationStateChanged(Qt::ApplicationState state);
200+ void onServiceDisconnected();
201+ void onServiceReconnected();
202 void onBufferingChanged();
203
204 protected:
205@@ -138,7 +140,6 @@
206
207 // Signals the proper QMediaPlayer::Error from a core::ubuntu::media::Error
208 void signalQMediaPlayerError(const core::ubuntu::media::Player::Error &error);
209-
210 void onError(const core::ubuntu::media::Player::Error &error);
211
212 inline QString playbackStatusStr(const core::ubuntu::media::Player::PlaybackStatus &status);
213@@ -148,6 +149,8 @@
214 core::Connection m_playbackStatusChangedConnection;
215 core::Connection m_errorConnection;
216 core::Connection m_endOfStreamConnection;
217+ core::Connection m_serviceDisconnectedConnection;
218+ core::Connection m_serviceReconnectedConnection;
219 core::Connection m_bufferingStatusChangedConnection;
220
221 AalMediaPlayerControl *m_mediaPlayerControl;
222
223=== modified file 'src/aal/aalmediaplaylistcontrol.cpp'
224--- src/aal/aalmediaplaylistcontrol.cpp 2016-01-13 16:45:28 +0000
225+++ src/aal/aalmediaplaylistcontrol.cpp 2016-07-20 21:27:06 +0000
226@@ -49,7 +49,6 @@
227
228 AalMediaPlaylistControl::~AalMediaPlaylistControl()
229 {
230- qDebug() << Q_FUNC_INFO;
231 disconnect_signals();
232 }
233
234@@ -180,7 +179,12 @@
235 QMediaPlaylist::PlaybackMode AalMediaPlaylistControl::playbackMode() const
236 {
237 QMediaPlaylist::PlaybackMode currentMode = QMediaPlaylist::Sequential;
238- const auto loopStatus = m_hubPlayerSession->loop_status();
239+ media::Player::LoopStatus loopStatus = media::Player::LoopStatus::none;
240+ try {
241+ loopStatus = m_hubPlayerSession->loop_status();
242+ } catch (const std::runtime_error &e) {
243+ qWarning() << "Failed to go to get loop_status: " << e.what();
244+ }
245 switch (loopStatus)
246 {
247 case media::Player::LoopStatus::none:
248@@ -196,10 +200,14 @@
249 qWarning() << "Unknown loop status: " << loopStatus;
250 }
251
252- // Shuffle overrides loopStatus since in the media-hub API random is not part of loop_status
253- // like it's all one for QMediaPlaylist::PlaybackMode
254- if (m_hubPlayerSession->shuffle())
255- currentMode = QMediaPlaylist::Random;
256+ try {
257+ // Shuffle overrides loopStatus since in the media-hub API random is not part of loop_status
258+ // like it's all one for QMediaPlaylist::PlaybackMode
259+ if (m_hubPlayerSession->shuffle())
260+ currentMode = QMediaPlaylist::Random;
261+ } catch (const std::runtime_error &e) {
262+ qWarning() << "Failed to get current shuffle mode: " << e.what();
263+ }
264
265 return currentMode;
266 }
267@@ -211,34 +219,58 @@
268 {
269 case QMediaPlaylist::CurrentItemOnce:
270 qDebug() << "PlaybackMode: CurrentItemOnce";
271- m_hubPlayerSession->shuffle() = false;
272+ try {
273+ m_hubPlayerSession->shuffle() = false;
274+ } catch (const std::runtime_error &e) {
275+ qWarning() << "Failed to set shuffle mode: " << e.what();
276+ }
277 qWarning() << "No media-hub equivalent for QMediaPlaylist::CurrentItemOnce";
278 break;
279 case QMediaPlaylist::CurrentItemInLoop:
280 qDebug() << "PlaybackMode: CurrentItemInLoop";
281- m_hubPlayerSession->shuffle() = false;
282- m_hubPlayerSession->loop_status() = media::Player::LoopStatus::track;
283+ try {
284+ m_hubPlayerSession->shuffle() = false;
285+ m_hubPlayerSession->loop_status() = media::Player::LoopStatus::track;
286+ } catch (const std::runtime_error &e) {
287+ qWarning() << "Failed to set shuffle mode/loop_status: " << e.what();
288+ }
289 break;
290 case QMediaPlaylist::Sequential:
291 qDebug() << "PlaybackMode: Sequential";
292- m_hubPlayerSession->shuffle() = false;
293- m_hubPlayerSession->loop_status() = media::Player::LoopStatus::none;
294+ try {
295+ m_hubPlayerSession->shuffle() = false;
296+ m_hubPlayerSession->loop_status() = media::Player::LoopStatus::none;
297+ } catch (const std::runtime_error &e) {
298+ qWarning() << "Failed to set shuffle mode/loop_status: " << e.what();
299+ }
300 break;
301 case QMediaPlaylist::Loop:
302 qDebug() << "PlaybackMode: Loop";
303- m_hubPlayerSession->shuffle() = false;
304- m_hubPlayerSession->loop_status() = media::Player::LoopStatus::playlist;
305+ try {
306+ m_hubPlayerSession->shuffle() = false;
307+ m_hubPlayerSession->loop_status() = media::Player::LoopStatus::playlist;
308+ } catch (const std::runtime_error &e) {
309+ qWarning() << "Failed to set shuffle mode/loop_status: " << e.what();
310+ }
311 break;
312 case QMediaPlaylist::Random:
313 qDebug() << "PlaybackMode: Random";
314- m_hubPlayerSession->shuffle() = true;
315- // FIXME: Until pad.lv/1518157 (RandomAndLoop playbackMode) is
316- // fixed set Random to be always looping due to pad.lv/1531296
317- m_hubPlayerSession->loop_status() = media::Player::LoopStatus::playlist;
318+ try {
319+ m_hubPlayerSession->shuffle() = true;
320+ // FIXME: Until pad.lv/1518157 (RandomAndLoop playbackMode) is
321+ // fixed set Random to be always looping due to pad.lv/1531296
322+ m_hubPlayerSession->loop_status() = media::Player::LoopStatus::playlist;
323+ } catch (const std::runtime_error &e) {
324+ qWarning() << "Failed to set shuffle mode/loop_status: " << e.what();
325+ }
326 break;
327 default:
328 qWarning() << "Unknown playback mode: " << mode;
329- m_hubPlayerSession->shuffle() = false;
330+ try {
331+ m_hubPlayerSession->shuffle() = false;
332+ } catch (const std::runtime_error &e) {
333+ qWarning() << "Failed to get current shuffle mode: " << e.what();
334+ }
335 }
336
337 Q_EMIT playbackModeChanged(mode);
338@@ -313,7 +345,11 @@
339 if (playbackMode() == QMediaPlaylist::Sequential)
340 {
341 qDebug() << "Repeat is off, so stopping playback";
342- m_hubPlayerSession->stop();
343+ try {
344+ m_hubPlayerSession->stop();
345+ } catch (std::runtime_error &e) {
346+ qWarning() << "FATAL: Failed to stop playback:" << e.what();
347+ }
348 }
349 }
350 }
351
352=== modified file 'src/aal/aalmediaplaylistprovider.cpp'
353--- src/aal/aalmediaplaylistprovider.cpp 2016-02-22 16:10:09 +0000
354+++ src/aal/aalmediaplaylistprovider.cpp 2016-07-20 21:27:06 +0000
355@@ -48,7 +48,6 @@
356
357 AalMediaPlaylistProvider::~AalMediaPlaylistProvider()
358 {
359- qDebug() << Q_FUNC_INFO;
360 disconnect_signals();
361 }
362
363@@ -414,9 +413,8 @@
364
365 try {
366 m_hubTrackList = m_hubPlayerSession->track_list();
367- }
368- catch (std::runtime_error &e) {
369- qWarning() << "FATAL: Failed to retrieve the current player session TrackList: " << e.what();
370+ } catch (std::runtime_error &e) {
371+ qWarning() << "FATAL: Failed to retrieve the current player session TrackList:" << e.what();
372 }
373
374 /* Disconnect first to avoid duplicated calls */
375
376=== modified file 'src/aal/aalvideorenderercontrol.cpp'
377--- src/aal/aalvideorenderercontrol.cpp 2015-06-18 16:17:21 +0000
378+++ src/aal/aalvideorenderercontrol.cpp 2016-07-20 21:27:06 +0000
379@@ -103,7 +103,8 @@
380 #endif
381 {
382 // Get notified when qtvideo-node creates a GL texture
383- connect(SharedSignal::instance(), SIGNAL(textureCreated(unsigned int)), this, SLOT(onTextureCreated(unsigned int)));
384+ connect(SharedSignal::instance(), SIGNAL(textureCreated(unsigned int)),
385+ this, SLOT(onTextureCreated(unsigned int)));
386 connect(SharedSignal::instance(), SIGNAL(glConsumerSet()), this, SLOT(onGLConsumerSet()));
387 connect(m_service, SIGNAL(playbackComplete()), this, SLOT(playbackComplete()));
388 }
389@@ -274,6 +275,12 @@
390 if (m_textureId == 0) {
391 m_textureId = static_cast<GLuint>(textureID);
392 m_videoSink = m_service->createVideoSink(textureID);
393+ if (not m_videoSink)
394+ {
395+ qWarning() << "Failed to create a new video sink with texture ID ("
396+ << textureID << "), m_videoSink is a nullptr";
397+ return;
398+ }
399
400 // Connect callback so that frames are rendered after decoding
401 m_frameAvailableConnection.reset(new core::Connection(m_videoSink->frame_available().connect(
402
403=== modified file 'tests/unit/service.cpp'
404--- tests/unit/service.cpp 2016-05-03 14:41:12 +0000
405+++ tests/unit/service.cpp 2016-07-20 21:27:06 +0000
406@@ -58,6 +58,18 @@
407 {
408 }
409
410+const core::Signal<void>& TestService::service_disconnected() const
411+{
412+ static const core::Signal<void> s;
413+ return s;
414+}
415+
416+const core::Signal<void>& TestService::service_reconnected() const
417+{
418+ static const core::Signal<void> s;
419+ return s;
420+}
421+
422 }
423 }
424 }
425
426=== modified file 'tests/unit/service.h'
427--- tests/unit/service.h 2016-05-03 14:41:12 +0000
428+++ tests/unit/service.h 2016-07-20 21:27:06 +0000
429@@ -51,6 +51,9 @@
430 virtual std::shared_ptr<Player> create_fixed_session(const std::string& name, const Player::Configuration&);
431 virtual std::shared_ptr<Player> resume_session(Player::PlayerKey);
432 virtual void pause_other_sessions(Player::PlayerKey);
433+
434+ virtual const core::Signal<void>& service_disconnected() const;
435+ virtual const core::Signal<void>& service_reconnected() const;
436 };
437 }
438 }

Subscribers

People subscribed via source and target branches

to all changes: