Merge lp:~phablet-team/media-hub/fix-1458040 into lp:media-hub
- fix-1458040
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Nick Dedekind | ||||
Approved revision: | 147 | ||||
Merged at revision: | 145 | ||||
Proposed branch: | lp:~phablet-team/media-hub/fix-1458040 | ||||
Merge into: | lp:media-hub | ||||
Prerequisite: | lp:~phablet-team/media-hub/fix-1457129 | ||||
Diff against target: |
318 lines (+121/-36) 5 files modified
src/core/media/gstreamer/bus.h (+41/-10) src/core/media/gstreamer/engine.cpp (+56/-12) src/core/media/gstreamer/playbin.cpp (+15/-12) src/core/media/gstreamer/playbin.h (+4/-1) tests/unit-tests/test-gstreamer-engine.cpp (+5/-1) |
||||
To merge this branch: | bzr merge lp:~phablet-team/media-hub/fix-1458040 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Nick Dedekind (community) | Approve | ||
PS Jenkins bot | continuous-integration | Approve | |
Review via email:
|
Commit message
Handle a wider array of GStreamer errors so that .ogv files are reported as failing to play.
Description of the change
Handle a wider array of GStreamer errors so that .ogv files are reported as failing to play.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Nick Dedekind (nick-dedekind) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Nick Dedekind (nick-dedekind) wrote : | # |
Still seems to be stalling in the dash usage, but not in the mediaplayer-app. Not sure why though; checking the differences but either way it shouldn't be happening.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Jim Hodapp (jhodapp) wrote : | # |
> Feels a bit strange that we return "no_error" if there is an error. Should we
> not add a "unknown_error" to the Player error api?
> This seems likely to be the reason why this failed in the first place?
My limitation is the types of errors supported by QMediaPlayer: http://
- 144. By Jim Hodapp
-
Log any playbin errors to stderr
- 145. By Jim Hodapp
-
Merged with trunk
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:145
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
- 146. By Jim Hodapp
-
Don't use a sync_handler for GST_MESSAGE handling. Use an async one instead so that we aren't on the streaming thread's context. This prevents the pipeline from deadlocking after an error is reached.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:146
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
- 147. By Jim Hodapp
-
Use both the sync_handler and async bus watch to handle GST_MESSAGES, but only using the sync one where absolutely necessary since it can cause deadlocks when used in general.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:147
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Nick Dedekind (nick-dedekind) wrote : | # |
Looks good and does as said on tin.
Preview Diff
1 | === modified file 'src/core/media/gstreamer/bus.h' | |||
2 | --- src/core/media/gstreamer/bus.h 2014-10-23 14:42:59 +0000 | |||
3 | +++ src/core/media/gstreamer/bus.h 2015-06-01 21:35:00 +0000 | |||
4 | @@ -277,12 +277,38 @@ | |||
5 | 277 | 277 | ||
6 | 278 | auto thiz = static_cast<Bus*>(data); | 278 | auto thiz = static_cast<Bus*>(data); |
7 | 279 | Message message(msg); | 279 | Message message(msg); |
14 | 280 | thiz->on_new_message(message); | 280 | if (message.type == GST_MESSAGE_TAG || message.type == GST_MESSAGE_ASYNC_DONE) |
15 | 281 | 281 | thiz->on_new_message(message); | |
16 | 282 | return GST_BUS_DROP; | 282 | |
17 | 283 | } | 283 | return GST_BUS_PASS; |
18 | 284 | 284 | } | |
19 | 285 | Bus(GstBus* bus) : bus(bus) | 285 | |
20 | 286 | static gboolean bus_watch_handler( | ||
21 | 287 | GstBus* bus, | ||
22 | 288 | GstMessage* msg, | ||
23 | 289 | gpointer data) | ||
24 | 290 | { | ||
25 | 291 | (void) bus; | ||
26 | 292 | |||
27 | 293 | auto thiz = static_cast<Bus*>(data); | ||
28 | 294 | Message message(msg); | ||
29 | 295 | thiz->on_new_message_async(message); | ||
30 | 296 | |||
31 | 297 | return true; | ||
32 | 298 | } | ||
33 | 299 | |||
34 | 300 | Bus(GstBus* bus) : bus(bus), bus_watch_id(0) | ||
35 | 301 | { | ||
36 | 302 | set_bus(bus); | ||
37 | 303 | } | ||
38 | 304 | |||
39 | 305 | ~Bus() | ||
40 | 306 | { | ||
41 | 307 | g_source_remove(bus_watch_id); | ||
42 | 308 | gst_object_unref(bus); | ||
43 | 309 | } | ||
44 | 310 | |||
45 | 311 | void set_bus(GstBus* bus) | ||
46 | 286 | { | 312 | { |
47 | 287 | if (!bus) | 313 | if (!bus) |
48 | 288 | throw std::runtime_error("Cannot create Bus instance if underlying instance is NULL."); | 314 | throw std::runtime_error("Cannot create Bus instance if underlying instance is NULL."); |
49 | @@ -292,15 +318,20 @@ | |||
50 | 292 | Bus::sync_handler, | 318 | Bus::sync_handler, |
51 | 293 | this, | 319 | this, |
52 | 294 | nullptr); | 320 | nullptr); |
53 | 295 | } | ||
54 | 296 | 321 | ||
58 | 297 | ~Bus() | 322 | // Use a watch for most messages instead of the sync handler so that our context is not |
59 | 298 | { | 323 | // the same as the streaming thread, which can cause deadlocks in GStreamer |
60 | 299 | gst_object_unref(bus); | 324 | // if, for example, attempting to change the pipeline state from this context. |
61 | 325 | bus_watch_id = gst_bus_add_watch( | ||
62 | 326 | bus, | ||
63 | 327 | Bus::bus_watch_handler, | ||
64 | 328 | this); | ||
65 | 300 | } | 329 | } |
66 | 301 | 330 | ||
67 | 302 | GstBus* bus; | 331 | GstBus* bus; |
68 | 303 | core::Signal<Message> on_new_message; | 332 | core::Signal<Message> on_new_message; |
69 | 333 | core::Signal<Message> on_new_message_async; | ||
70 | 334 | guint bus_watch_id; | ||
71 | 304 | }; | 335 | }; |
72 | 305 | } | 336 | } |
73 | 306 | 337 | ||
74 | 307 | 338 | ||
75 | === modified file 'src/core/media/gstreamer/engine.cpp' | |||
76 | --- src/core/media/gstreamer/engine.cpp 2015-05-01 19:44:11 +0000 | |||
77 | +++ src/core/media/gstreamer/engine.cpp 2015-06-01 21:35:00 +0000 | |||
78 | @@ -75,53 +75,97 @@ | |||
79 | 75 | // Converts from a GStreamer GError to a media::Player:Error enum | 75 | // Converts from a GStreamer GError to a media::Player:Error enum |
80 | 76 | media::Player::Error from_gst_errorwarning(const gstreamer::Bus::Message::Detail::ErrorWarningInfo& ewi) | 76 | media::Player::Error from_gst_errorwarning(const gstreamer::Bus::Message::Detail::ErrorWarningInfo& ewi) |
81 | 77 | { | 77 | { |
82 | 78 | media::Player::Error ret_error = media::Player::Error::no_error; | ||
83 | 79 | |||
84 | 78 | if (g_strcmp0(g_quark_to_string(ewi.error->domain), "gst-core-error-quark") == 0) | 80 | if (g_strcmp0(g_quark_to_string(ewi.error->domain), "gst-core-error-quark") == 0) |
85 | 79 | { | 81 | { |
86 | 80 | switch (ewi.error->code) | 82 | switch (ewi.error->code) |
87 | 81 | { | 83 | { |
88 | 84 | case GST_CORE_ERROR_FAILED: | ||
89 | 85 | std::cerr << "** Encountered a GST_CORE_ERROR_FAILED" << std::endl; | ||
90 | 86 | ret_error = media::Player::Error::resource_error; | ||
91 | 87 | break; | ||
92 | 82 | case GST_CORE_ERROR_NEGOTIATION: | 88 | case GST_CORE_ERROR_NEGOTIATION: |
94 | 83 | return media::Player::Error::resource_error; | 89 | std::cerr << "** Encountered a GST_CORE_ERROR_NEGOTIATION" << std::endl; |
95 | 90 | ret_error = media::Player::Error::resource_error; | ||
96 | 91 | break; | ||
97 | 84 | case GST_CORE_ERROR_MISSING_PLUGIN: | 92 | case GST_CORE_ERROR_MISSING_PLUGIN: |
99 | 85 | return media::Player::Error::format_error; | 93 | std::cerr << "** Encountered a GST_CORE_ERROR_MISSING_PLUGIN" << std::endl; |
100 | 94 | ret_error = media::Player::Error::format_error; | ||
101 | 95 | break; | ||
102 | 86 | default: | 96 | default: |
104 | 87 | std::cerr << "Got an unhandled core error: '" | 97 | std::cerr << "** Encountered an unhandled core error: '" |
105 | 88 | << ewi.debug << "' (code: " << ewi.error->code << ")" << std::endl; | 98 | << ewi.debug << "' (code: " << ewi.error->code << ")" << std::endl; |
107 | 89 | return media::Player::Error::no_error; | 99 | ret_error = media::Player::Error::no_error; |
108 | 100 | break; | ||
109 | 90 | } | 101 | } |
110 | 91 | } | 102 | } |
111 | 92 | else if (g_strcmp0(g_quark_to_string(ewi.error->domain), "gst-resource-error-quark") == 0) | 103 | else if (g_strcmp0(g_quark_to_string(ewi.error->domain), "gst-resource-error-quark") == 0) |
112 | 93 | { | 104 | { |
113 | 94 | switch (ewi.error->code) | 105 | switch (ewi.error->code) |
114 | 95 | { | 106 | { |
115 | 107 | case GST_RESOURCE_ERROR_FAILED: | ||
116 | 108 | std::cerr << "** Encountered a GST_RESOURCE_ERROR_FAILED" << std::endl; | ||
117 | 109 | ret_error = media::Player::Error::resource_error; | ||
118 | 110 | break; | ||
119 | 96 | case GST_RESOURCE_ERROR_NOT_FOUND: | 111 | case GST_RESOURCE_ERROR_NOT_FOUND: |
120 | 112 | std::cerr << "** Encountered a GST_RESOURCE_ERROR_NOT_FOUND" << std::endl; | ||
121 | 113 | ret_error = media::Player::Error::resource_error; | ||
122 | 114 | break; | ||
123 | 97 | case GST_RESOURCE_ERROR_OPEN_READ: | 115 | case GST_RESOURCE_ERROR_OPEN_READ: |
124 | 116 | std::cerr << "** Encountered a GST_RESOURCE_ERROR_OPEN_READ" << std::endl; | ||
125 | 117 | ret_error = media::Player::Error::resource_error; | ||
126 | 118 | break; | ||
127 | 98 | case GST_RESOURCE_ERROR_OPEN_WRITE: | 119 | case GST_RESOURCE_ERROR_OPEN_WRITE: |
128 | 120 | std::cerr << "** Encountered a GST_RESOURCE_ERROR_OPEN_WRITE" << std::endl; | ||
129 | 121 | ret_error = media::Player::Error::resource_error; | ||
130 | 122 | break; | ||
131 | 99 | case GST_RESOURCE_ERROR_READ: | 123 | case GST_RESOURCE_ERROR_READ: |
132 | 124 | std::cerr << "** Encountered a GST_RESOURCE_ERROR_READ" << std::endl; | ||
133 | 125 | ret_error = media::Player::Error::resource_error; | ||
134 | 126 | break; | ||
135 | 100 | case GST_RESOURCE_ERROR_WRITE: | 127 | case GST_RESOURCE_ERROR_WRITE: |
137 | 101 | return media::Player::Error::resource_error; | 128 | std::cerr << "** Encountered a GST_RESOURCE_ERROR_WRITE" << std::endl; |
138 | 129 | ret_error = media::Player::Error::resource_error; | ||
139 | 130 | break; | ||
140 | 102 | case GST_RESOURCE_ERROR_NOT_AUTHORIZED: | 131 | case GST_RESOURCE_ERROR_NOT_AUTHORIZED: |
142 | 103 | return media::Player::Error::access_denied_error; | 132 | std::cerr << "** Encountered a GST_RESOURCE_ERROR_NOT_AUTHORIZED" << std::endl; |
143 | 133 | ret_error = media::Player::Error::access_denied_error; | ||
144 | 134 | break; | ||
145 | 104 | default: | 135 | default: |
147 | 105 | std::cerr << "Got an unhandled resource error: '" | 136 | std::cerr << "** Encountered an unhandled resource error: '" |
148 | 106 | << ewi.debug << "' (code: " << ewi.error->code << ")" << std::endl; | 137 | << ewi.debug << "' (code: " << ewi.error->code << ")" << std::endl; |
150 | 107 | return media::Player::Error::no_error; | 138 | ret_error = media::Player::Error::no_error; |
151 | 139 | break; | ||
152 | 108 | } | 140 | } |
153 | 109 | } | 141 | } |
154 | 110 | else if (g_strcmp0(g_quark_to_string(ewi.error->domain), "gst-stream-error-quark") == 0) | 142 | else if (g_strcmp0(g_quark_to_string(ewi.error->domain), "gst-stream-error-quark") == 0) |
155 | 111 | { | 143 | { |
156 | 112 | switch (ewi.error->code) | 144 | switch (ewi.error->code) |
157 | 113 | { | 145 | { |
158 | 146 | case GST_STREAM_ERROR_FAILED: | ||
159 | 147 | std::cerr << "** Encountered a GST_STREAM_ERROR_FAILED" << std::endl; | ||
160 | 148 | ret_error = media::Player::Error::resource_error; | ||
161 | 149 | break; | ||
162 | 114 | case GST_STREAM_ERROR_CODEC_NOT_FOUND: | 150 | case GST_STREAM_ERROR_CODEC_NOT_FOUND: |
163 | 151 | std::cerr << "** Encountered a GST_STREAM_ERROR_CODEC_NOT_FOUND" << std::endl; | ||
164 | 152 | ret_error = media::Player::Error::format_error; | ||
165 | 153 | break; | ||
166 | 115 | case GST_STREAM_ERROR_DECODE: | 154 | case GST_STREAM_ERROR_DECODE: |
168 | 116 | return media::Player::Error::format_error; | 155 | std::cerr << "** Encountered a GST_STREAM_ERROR_DECODE" << std::endl; |
169 | 156 | ret_error = media::Player::Error::format_error; | ||
170 | 157 | break; | ||
171 | 117 | default: | 158 | default: |
173 | 118 | std::cerr << "Got an unhandled stream error: '" | 159 | std::cerr << "** Encountered an unhandled stream error: '" |
174 | 119 | << ewi.debug << "' (code: " << ewi.error->code << ")" << std::endl; | 160 | << ewi.debug << "' (code: " << ewi.error->code << ")" << std::endl; |
176 | 120 | return media::Player::Error::no_error; | 161 | ret_error = media::Player::Error::no_error; |
177 | 162 | break; | ||
178 | 121 | } | 163 | } |
179 | 122 | } | 164 | } |
180 | 123 | 165 | ||
182 | 124 | return media::Player::Error::no_error; | 166 | std::cout << "Resetting playbin pipeline after unrecoverable error" << std::endl; |
183 | 167 | playbin.reset(); | ||
184 | 168 | return ret_error; | ||
185 | 125 | } | 169 | } |
186 | 126 | 170 | ||
187 | 127 | void on_playbin_error(const gstreamer::Bus::Message::Detail::ErrorWarningInfo& ewi) | 171 | void on_playbin_error(const gstreamer::Bus::Message::Detail::ErrorWarningInfo& ewi) |
188 | 128 | 172 | ||
189 | === modified file 'src/core/media/gstreamer/playbin.cpp' | |||
190 | --- src/core/media/gstreamer/playbin.cpp 2015-05-01 19:44:11 +0000 | |||
191 | +++ src/core/media/gstreamer/playbin.cpp 2015-06-01 21:35:00 +0000 | |||
192 | @@ -92,34 +92,34 @@ | |||
193 | 92 | bus{gst_element_get_bus(pipeline)}, | 92 | bus{gst_element_get_bus(pipeline)}, |
194 | 93 | file_type(MEDIA_FILE_TYPE_NONE), | 93 | file_type(MEDIA_FILE_TYPE_NONE), |
195 | 94 | video_sink(nullptr), | 94 | video_sink(nullptr), |
198 | 95 | on_new_message_connection( | 95 | on_new_message_connection_async( |
199 | 96 | bus.on_new_message.connect( | 96 | bus.on_new_message_async.connect( |
200 | 97 | std::bind( | 97 | std::bind( |
202 | 98 | &Playbin::on_new_message, | 98 | &Playbin::on_new_message_async, |
203 | 99 | this, | 99 | this, |
204 | 100 | std::placeholders::_1))), | 100 | std::placeholders::_1))), |
205 | 101 | is_seeking(false), | 101 | is_seeking(false), |
206 | 102 | previous_position(0), | 102 | previous_position(0), |
207 | 103 | player_lifetime(media::Player::Lifetime::normal), | 103 | player_lifetime(media::Player::Lifetime::normal), |
209 | 104 | is_eos(false) | 104 | is_eos(false), |
210 | 105 | about_to_finish_handler_id(0), | ||
211 | 106 | source_setup_handler_id(0) | ||
212 | 105 | { | 107 | { |
213 | 106 | if (!pipeline) | 108 | if (!pipeline) |
214 | 107 | throw std::runtime_error("Could not create pipeline for playbin."); | 109 | throw std::runtime_error("Could not create pipeline for playbin."); |
215 | 108 | 110 | ||
216 | 109 | is_eos = false; | ||
217 | 110 | |||
218 | 111 | // Add audio and/or video sink elements depending on environment variables | 111 | // Add audio and/or video sink elements depending on environment variables |
219 | 112 | // being set or not set | 112 | // being set or not set |
220 | 113 | setup_pipeline_for_audio_video(); | 113 | setup_pipeline_for_audio_video(); |
221 | 114 | 114 | ||
223 | 115 | g_signal_connect( | 115 | about_to_finish_handler_id = g_signal_connect( |
224 | 116 | pipeline, | 116 | pipeline, |
225 | 117 | "about-to-finish", | 117 | "about-to-finish", |
226 | 118 | G_CALLBACK(about_to_finish), | 118 | G_CALLBACK(about_to_finish), |
227 | 119 | this | 119 | this |
228 | 120 | ); | 120 | ); |
229 | 121 | 121 | ||
231 | 122 | g_signal_connect( | 122 | source_setup_handler_id = g_signal_connect( |
232 | 123 | pipeline, | 123 | pipeline, |
233 | 124 | "source-setup", | 124 | "source-setup", |
234 | 125 | G_CALLBACK(source_setup), | 125 | G_CALLBACK(source_setup), |
235 | @@ -129,6 +129,9 @@ | |||
236 | 129 | 129 | ||
237 | 130 | gstreamer::Playbin::~Playbin() | 130 | gstreamer::Playbin::~Playbin() |
238 | 131 | { | 131 | { |
239 | 132 | g_signal_handler_disconnect(pipeline, about_to_finish_handler_id); | ||
240 | 133 | g_signal_handler_disconnect(pipeline, source_setup_handler_id); | ||
241 | 134 | |||
242 | 132 | if (pipeline) | 135 | if (pipeline) |
243 | 133 | gst_object_unref(pipeline); | 136 | gst_object_unref(pipeline); |
244 | 134 | } | 137 | } |
245 | @@ -167,7 +170,7 @@ | |||
246 | 167 | file_type = MEDIA_FILE_TYPE_NONE; | 170 | file_type = MEDIA_FILE_TYPE_NONE; |
247 | 168 | } | 171 | } |
248 | 169 | 172 | ||
250 | 170 | void gstreamer::Playbin::on_new_message(const Bus::Message& message) | 173 | void gstreamer::Playbin::on_new_message_async(const Bus::Message& message) |
251 | 171 | { | 174 | { |
252 | 172 | switch(message.type) | 175 | switch(message.type) |
253 | 173 | { | 176 | { |
254 | @@ -180,6 +183,9 @@ | |||
255 | 180 | case GST_MESSAGE_INFO: | 183 | case GST_MESSAGE_INFO: |
256 | 181 | signals.on_info(message.detail.error_warning_info); | 184 | signals.on_info(message.detail.error_warning_info); |
257 | 182 | break; | 185 | break; |
258 | 186 | case GST_MESSAGE_STATE_CHANGED: | ||
259 | 187 | signals.on_state_changed(std::make_pair(message.detail.state_changed, message.source)); | ||
260 | 188 | break; | ||
261 | 183 | case GST_MESSAGE_TAG: | 189 | case GST_MESSAGE_TAG: |
262 | 184 | { | 190 | { |
263 | 185 | gchar *orientation; | 191 | gchar *orientation; |
264 | @@ -193,9 +199,6 @@ | |||
265 | 193 | signals.on_tag_available(message.detail.tag); | 199 | signals.on_tag_available(message.detail.tag); |
266 | 194 | } | 200 | } |
267 | 195 | break; | 201 | break; |
268 | 196 | case GST_MESSAGE_STATE_CHANGED: | ||
269 | 197 | signals.on_state_changed(std::make_pair(message.detail.state_changed, message.source)); | ||
270 | 198 | break; | ||
271 | 199 | case GST_MESSAGE_ASYNC_DONE: | 202 | case GST_MESSAGE_ASYNC_DONE: |
272 | 200 | if (is_seeking) | 203 | if (is_seeking) |
273 | 201 | { | 204 | { |
274 | 202 | 205 | ||
275 | === modified file 'src/core/media/gstreamer/playbin.h' | |||
276 | --- src/core/media/gstreamer/playbin.h 2015-05-01 19:44:11 +0000 | |||
277 | +++ src/core/media/gstreamer/playbin.h 2015-06-01 21:35:00 +0000 | |||
278 | @@ -71,6 +71,7 @@ | |||
279 | 71 | void reset_pipeline(); | 71 | void reset_pipeline(); |
280 | 72 | 72 | ||
281 | 73 | void on_new_message(const Bus::Message& message); | 73 | void on_new_message(const Bus::Message& message); |
282 | 74 | void on_new_message_async(const Bus::Message& message); | ||
283 | 74 | 75 | ||
284 | 75 | gstreamer::Bus& message_bus(); | 76 | gstreamer::Bus& message_bus(); |
285 | 76 | 77 | ||
286 | @@ -112,12 +113,14 @@ | |||
287 | 112 | gstreamer::Bus bus; | 113 | gstreamer::Bus bus; |
288 | 113 | MediaFileType file_type; | 114 | MediaFileType file_type; |
289 | 114 | GstElement* video_sink; | 115 | GstElement* video_sink; |
291 | 115 | core::Connection on_new_message_connection; | 116 | core::Connection on_new_message_connection_async; |
292 | 116 | bool is_seeking; | 117 | bool is_seeking; |
293 | 117 | mutable uint64_t previous_position; | 118 | mutable uint64_t previous_position; |
294 | 118 | core::ubuntu::media::Player::HeadersType request_headers; | 119 | core::ubuntu::media::Player::HeadersType request_headers; |
295 | 119 | core::ubuntu::media::Player::Lifetime player_lifetime; | 120 | core::ubuntu::media::Player::Lifetime player_lifetime; |
296 | 120 | bool is_eos; | 121 | bool is_eos; |
297 | 122 | gulong about_to_finish_handler_id; | ||
298 | 123 | gulong source_setup_handler_id; | ||
299 | 121 | struct | 124 | struct |
300 | 122 | { | 125 | { |
301 | 123 | core::Signal<void> about_to_finish; | 126 | core::Signal<void> about_to_finish; |
302 | 124 | 127 | ||
303 | === modified file 'tests/unit-tests/test-gstreamer-engine.cpp' | |||
304 | --- tests/unit-tests/test-gstreamer-engine.cpp 2015-04-03 22:52:08 +0000 | |||
305 | +++ tests/unit-tests/test-gstreamer-engine.cpp 2015-06-01 21:35:00 +0000 | |||
306 | @@ -411,7 +411,11 @@ | |||
307 | 411 | ASSERT_TRUE(test::copy_test_media_file_to("test.mp3", test_file)); | 411 | ASSERT_TRUE(test::copy_test_media_file_to("test.mp3", test_file)); |
308 | 412 | 412 | ||
309 | 413 | gstreamer::Engine engine; | 413 | gstreamer::Engine engine; |
311 | 414 | auto md = engine.meta_data_extractor()->meta_data_for_track_with_uri(test_file_uri); | 414 | |
312 | 415 | core::ubuntu::media::Track::MetaData md; | ||
313 | 416 | ASSERT_NO_THROW({ | ||
314 | 417 | md = engine.meta_data_extractor()->meta_data_for_track_with_uri(test_file_uri); | ||
315 | 418 | }); | ||
316 | 415 | 419 | ||
317 | 416 | if (0 < md.count(xesam::Album::name)) | 420 | if (0 < md.count(xesam::Album::name)) |
318 | 417 | EXPECT_EQ("Test", md.get(xesam::Album::name)); | 421 | EXPECT_EQ("Test", md.get(xesam::Album::name)); |
Feels a bit strange that we return "no_error" if there is an error. Should we not add a "unknown_error" to the Player error api?
This seems likely to be the reason why this failed in the first place?