Merge lp:~phablet-team/media-hub/fix-black-video-bugs into lp:media-hub

Proposed by Jim Hodapp
Status: Merged
Approved by: Alfonso Sanchez-Beato
Approved revision: 148
Merged at revision: 147
Proposed branch: lp:~phablet-team/media-hub/fix-black-video-bugs
Merge into: lp:media-hub
Diff against target: 89 lines (+19/-7)
2 files modified
src/core/media/gstreamer/playbin.cpp (+17/-6)
src/core/media/gstreamer/playbin.h (+2/-1)
To merge this branch: bzr merge lp:~phablet-team/media-hub/fix-black-video-bugs
Reviewer Review Type Date Requested Status
Alfonso Sanchez-Beato Approve
PS Jenkins bot continuous-integration Approve
Review via email: mp+262096@code.launchpad.net

Commit message

Only send the video_dimensions_changed signal if the dimensions actually changed. Also, no longer need is_eos flag.

Description of the change

Only send the video_dimensions_changed signal if the dimensions actually changed. Also, no longer need is_eos flag.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alfonso Sanchez-Beato (alfonsosanchezbeato) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/core/media/gstreamer/playbin.cpp'
--- src/core/media/gstreamer/playbin.cpp 2015-06-01 21:34:46 +0000
+++ src/core/media/gstreamer/playbin.cpp 2015-06-16 14:42:38 +0000
@@ -100,8 +100,10 @@
100 std::placeholders::_1))),100 std::placeholders::_1))),
101 is_seeking(false),101 is_seeking(false),
102 previous_position(0),102 previous_position(0),
103 cached_video_dimensions{
104 core::ubuntu::media::video::Height{0},
105 core::ubuntu::media::video::Width{0}},
103 player_lifetime(media::Player::Lifetime::normal),106 player_lifetime(media::Player::Lifetime::normal),
104 is_eos(false),
105 about_to_finish_handler_id(0),107 about_to_finish_handler_id(0),
106 source_setup_handler_id(0)108 source_setup_handler_id(0)
107{109{
@@ -208,7 +210,6 @@
208 }210 }
209 break;211 break;
210 case GST_MESSAGE_EOS:212 case GST_MESSAGE_EOS:
211 is_eos = true;
212 signals.on_end_of_stream();213 signals.on_end_of_stream();
213 default:214 default:
214 break;215 break;
@@ -450,14 +451,15 @@
450 }451 }
451452
452 // We only should query the pipeline if we actually succeeded in453 // We only should query the pipeline if we actually succeeded in
453 // setting the requested state. Also don't send on_video_dimensions_changed454 // setting the requested state.
454 // signal during EOS.455 if (result && new_state == GST_STATE_PLAYING)
455 if (result && new_state == GST_STATE_PLAYING && !is_eos)
456 {456 {
457 // Get the video height/width from the video sink457 // Get the video height/width from the video sink
458 try458 try
459 {459 {
460 signals.on_video_dimensions_changed(get_video_dimensions());460 const core::ubuntu::media::video::Dimensions new_dimensions = get_video_dimensions();
461 emit_video_dimensions_changed_if_changed(new_dimensions);
462 cached_video_dimensions = new_dimensions;
461 }463 }
462 catch (const std::exception& e)464 catch (const std::exception& e)
463 {465 {
@@ -499,6 +501,7 @@
499 uint32_t video_width = 0, video_height = 0;501 uint32_t video_width = 0, video_height = 0;
500 g_object_get (video_sink, "height", &video_height, nullptr);502 g_object_get (video_sink, "height", &video_height, nullptr);
501 g_object_get (video_sink, "width", &video_width, nullptr);503 g_object_get (video_sink, "width", &video_width, nullptr);
504
502 // TODO(tvoss): We should probably check here if width and height are valid.505 // TODO(tvoss): We should probably check here if width and height are valid.
503 return core::ubuntu::media::video::Dimensions506 return core::ubuntu::media::video::Dimensions
504 {507 {
@@ -507,6 +510,14 @@
507 };510 };
508}511}
509512
513void gstreamer::Playbin::emit_video_dimensions_changed_if_changed(const core::ubuntu::media::video::Dimensions &new_dimensions)
514{
515 // Only signal the application layer if the dimensions have in fact changed. This might happen
516 // if reusing the same media-hub session to play two different video sources.
517 if (new_dimensions != cached_video_dimensions)
518 signals.on_video_dimensions_changed(new_dimensions);
519}
520
510std::string gstreamer::Playbin::get_file_content_type(const std::string& uri) const521std::string gstreamer::Playbin::get_file_content_type(const std::string& uri) const
511{522{
512 if (uri.empty())523 if (uri.empty())
513524
=== modified file 'src/core/media/gstreamer/playbin.h'
--- src/core/media/gstreamer/playbin.h 2015-06-01 21:34:46 +0000
+++ src/core/media/gstreamer/playbin.h 2015-06-16 14:42:38 +0000
@@ -101,6 +101,7 @@
101 bool seek(const std::chrono::microseconds& ms);101 bool seek(const std::chrono::microseconds& ms);
102102
103 core::ubuntu::media::video::Dimensions get_video_dimensions() const;103 core::ubuntu::media::video::Dimensions get_video_dimensions() const;
104 void emit_video_dimensions_changed_if_changed(const core::ubuntu::media::video::Dimensions &new_dimensions);
104105
105 std::string get_file_content_type(const std::string& uri) const;106 std::string get_file_content_type(const std::string& uri) const;
106107
@@ -116,9 +117,9 @@
116 core::Connection on_new_message_connection_async;117 core::Connection on_new_message_connection_async;
117 bool is_seeking;118 bool is_seeking;
118 mutable uint64_t previous_position;119 mutable uint64_t previous_position;
120 core::ubuntu::media::video::Dimensions cached_video_dimensions;
119 core::ubuntu::media::Player::HeadersType request_headers;121 core::ubuntu::media::Player::HeadersType request_headers;
120 core::ubuntu::media::Player::Lifetime player_lifetime;122 core::ubuntu::media::Player::Lifetime player_lifetime;
121 bool is_eos;
122 gulong about_to_finish_handler_id;123 gulong about_to_finish_handler_id;
123 gulong source_setup_handler_id;124 gulong source_setup_handler_id;
124 struct125 struct

Subscribers

People subscribed via source and target branches

to all changes: