[Merge] lp:~alfonsosanchezbeato/media-hub/get-selected-streams into lp:media-hub

Jim Hodapp jim.hodapp at canonical.com
Tue Aug 4 01:52:08 UTC 2015


Review: Needs Fixing code

A few suggestions below. I like the general direction of this approach. Are you planning on adding anything else to it or is this it?

Diff comments:

> 
> === modified file 'src/core/media/gstreamer/engine.cpp'
> --- src/core/media/gstreamer/engine.cpp	2015-06-01 16:30:59 +0000
> +++ src/core/media/gstreamer/engine.cpp	2015-07-29 15:19:28 +0000
> @@ -67,8 +67,22 @@
>      {
>          if (p.second == "playbin")
>          {
> -            std::cout << "State changed on playbin: " << p.first.new_state << std::endl;
> -            playback_status_changed(gst_state_to_player_status(p.first));
> +            std::cout << "State changed on playbin: "
> +                      << gst_element_state_get_name(p.first.new_state) << std::endl;
> +            auto status = gst_state_to_player_status(p.first);
> +            /*
> +             * When state moves to "paused" the pipeline is already set. We check that we
> +             * have streams to play.
> +             */
> +            if (status == media::Player::PlaybackStatus::paused &&
> +                    !playbin.can_play_streams()) {
> +                std::cerr << "** Cannot play: some codecs are missing" << std::endl;
> +                playbin.reset();
> +                media::Player::Error e = media::Player::Error::format_error;

const

> +                error(e);
> +            } else {
> +                playback_status_changed(status);
> +            }
>          }
>      }
>  
> 
> === modified file 'src/core/media/gstreamer/playbin.cpp'
> --- src/core/media/gstreamer/playbin.cpp	2015-07-24 14:54:19 +0000
> +++ src/core/media/gstreamer/playbin.cpp	2015-07-29 15:19:28 +0000
> @@ -179,6 +185,41 @@
>      file_type = MEDIA_FILE_TYPE_NONE;
>  }
>  
> +void gstreamer::Playbin::process_message_element(GstMessage *message)
> +{
> +    if (!gst_is_missing_plugin_message(message))
> +      return;
> +
> +    gchar *desc = gst_missing_plugin_message_get_description(message);
> +    std::cerr << "Missing plugin: " << desc << std::endl;
> +    g_free(desc);
> +
> +    const GstStructure *msg_data = gst_message_get_structure(message);
> +    if (g_strcmp0("decoder", gst_structure_get_string(msg_data, "type")) != 0)
> +        return;
> +
> +    GstCaps *caps;
> +    if (!gst_structure_get(msg_data, "detail", GST_TYPE_CAPS, &caps, NULL)) {
> +        std::cerr << __PRETTY_FUNCTION__ << ": No detail" << std::endl;
> +        return;
> +    }
> +
> +    GstStructure *caps_data;
> +    caps_data = gst_caps_get_structure(caps, 0);

Declare and initialize caps_data on the same line in this instance.

> +    if (!caps_data) {
> +        std::cerr << __PRETTY_FUNCTION__ << ": No caps data" << std::endl;
> +        return;
> +    }
> +
> +    const gchar *mime = gst_structure_get_name(caps_data);
> +    if (strstr(mime, "audio"))
> +        is_missing_audio_codec = true;
> +    else if (strstr(mime, "video"))
> +        is_missing_video_codec = true;
> +
> +    std::cout << "Missing decoder for " << mime << std::endl;
> +}
> +
>  void gstreamer::Playbin::on_new_message_async(const Bus::Message& message)
>  {
>      switch(message.type)


-- 
https://code.launchpad.net/~alfonsosanchezbeato/media-hub/get-selected-streams/+merge/266251
Your team Ubuntu Phablet Team is subscribed to branch lp:media-hub.



More information about the Ubuntu-reviews mailing list