[Merge] lp:~justinmcp/media-hub/oxide-support into lp:~phablet-team/media-hub/oxide-support

Jim Hodapp jim.hodapp at canonical.com
Fri Sep 12 13:45:24 UTC 2014


Review: Needs Fixing code

Just one fix needed.

Diff comments:

> === modified file 'src/core/media/engine.h'
> --- src/core/media/engine.h	2014-04-25 17:53:00 +0000
> +++ src/core/media/engine.h	2014-09-03 04:19:37 +0000
> @@ -98,6 +98,9 @@
>      virtual const core::Property<State>& state() const = 0;
>  
>      virtual bool open_resource_for_uri(const Track::UriType& uri) = 0;
> +    virtual bool open_resource_for_uri(const core::ubuntu::media::Track::UriType& uri,
> +                                const std::string& cookies,
> +                                const std::string& user_agent) = 0;
>      virtual void create_video_sink(uint32_t texture_id) = 0;
>  
>      virtual bool play() = 0;
> 
> === modified file 'src/core/media/gstreamer/bus.h'
> --- src/core/media/gstreamer/bus.h	2014-02-12 15:53:57 +0000
> +++ src/core/media/gstreamer/bus.h	2014-09-03 04:19:37 +0000
> @@ -208,7 +208,9 @@
>                  break;
>              case GST_MESSAGE_ANY:
>                  break;
> -            }
> +            default:
> +                break;
> +            }						
>          }
>  
>          GstMessage* message;
> 
> === modified file 'src/core/media/gstreamer/engine.cpp'
> --- src/core/media/gstreamer/engine.cpp	2014-04-25 17:53:00 +0000
> +++ src/core/media/gstreamer/engine.cpp	2014-09-03 04:19:37 +0000
> @@ -271,6 +271,13 @@
>      return true;
>  }
>  
> +bool gstreamer::Engine::open_resource_for_uri(const media::Track::UriType& uri, const std::string& cookies,
> +        const std::string& user_agent)
> +{
> +    d->playbin.set_uri(uri, cookies, user_agent);
> +    return true;
> +}
> +
>  void gstreamer::Engine::create_video_sink(uint32_t texture_id)
>  {
>      d->playbin.create_video_sink(texture_id);
> 
> === modified file 'src/core/media/gstreamer/engine.h'
> --- src/core/media/gstreamer/engine.h	2014-04-25 17:53:00 +0000
> +++ src/core/media/gstreamer/engine.h	2014-09-03 04:19:37 +0000
> @@ -33,6 +33,9 @@
>      const core::Property<State>& state() const;
>  
>      bool open_resource_for_uri(const core::ubuntu::media::Track::UriType& uri);
> +    bool open_resource_for_uri(const core::ubuntu::media::Track::UriType& uri,
> +                                const std::string& cookies,
> +                                const std::string& user_agent);
>      void create_video_sink(uint32_t texture_id);
>  
>      bool play();
> 
> === modified file 'src/core/media/gstreamer/playbin.h'
> --- src/core/media/gstreamer/playbin.h	2014-04-25 21:23:10 +0000
> +++ src/core/media/gstreamer/playbin.h	2014-09-03 04:19:37 +0000
> @@ -64,6 +64,13 @@
>          thiz->signals.about_to_finish();
>      }
>  
> +    static void source_setup(GstElement*,
> +                             GstElement *source,
> +                             gpointer user_data)
> +    {
> +        static_cast<Playbin*>(user_data)->setup_source(source);
> +    }
> +
>      Playbin()
>          : pipeline(gst_element_factory_make("playbin", pipeline_name().c_str())),
>            bus{gst_element_get_bus(pipeline)},
> @@ -90,6 +97,13 @@
>                      this
>                      );
>  
> +        g_signal_connect(
> +                    pipeline,
> +                    "source-setup",
> +                    G_CALLBACK(source_setup),
> +                    this
> +                    );
> +
>          // When a client of media-hub dies, call on_client_died
>          decoding_service_set_client_death_cb(&Playbin::on_client_died_cb, static_cast<void*>(this));
>      }
> @@ -258,13 +272,37 @@
>          return static_cast<uint64_t>(dur);
>      }
>  
> -    void set_uri(const std::string& uri)
> +    void set_uri(const std::string& uri,
> +                 const std::string& cookies = std::string(),
> +                 const std::string& user_agent = std::string())
>      {
>          g_object_set(pipeline, "uri", uri.c_str(), NULL);
>          if (is_video_file(uri))
>              file_type = MEDIA_FILE_TYPE_VIDEO;
>          else if (is_audio_file(uri))
>              file_type = MEDIA_FILE_TYPE_AUDIO;
> +
> +        request_cookies = cookies;
> +        request_user_agent = user_agent;
> +    }
> +
> +    void setup_source(GstElement *source)
> +    {

Please add a check to make sure that source is not NULL

> +        if (!request_cookies.empty()) {
> +            if (g_object_class_find_property(G_OBJECT_GET_CLASS(source),
> +                                             "cookies") != NULL) {
> +                gchar ** cookies = g_strsplit(request_cookies.c_str(), ";", 0);
> +                g_object_set(source, "cookies", cookies, NULL);
> +                g_strfreev(cookies);
> +            }
> +        }
> +
> +        if (!request_user_agent.empty()) {
> +            if (g_object_class_find_property(G_OBJECT_GET_CLASS(source),
> +                                             "user-agent") != NULL) {
> +                g_object_set(source, "user-agent", request_user_agent.c_str(), NULL);
> +            }
> +        }
>      }
>  
>      std::string uri() const
> @@ -400,6 +438,8 @@
>      SurfaceTextureClientHybris stc_hybris;
>      core::Connection on_new_message_connection;
>      bool is_seeking;
> +    std::string request_cookies;
> +    std::string request_user_agent;
>      struct
>      {
>          core::Signal<void> about_to_finish;
> 
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp	2014-07-22 15:03:36 +0000
> +++ src/core/media/player_implementation.cpp	2014-09-03 04:19:37 +0000
> @@ -279,11 +279,7 @@
>  bool media::PlayerImplementation::open_uri(const Track::UriType& uri, const std::string& cookies,
>          const std::string& user_agent)
>  {
> -    // TODO: Implement this for proper Oxide media playback support
> -    (void)uri;
> -    (void)cookies;
> -    (void)user_agent;
> -    return true;
> +    return d->engine->open_resource_for_uri(uri, cookies, user_agent);
>  }
>  
>  void media::PlayerImplementation::create_video_sink(uint32_t texture_id)
> 
> === modified file 'src/core/media/player_stub.cpp'
> --- src/core/media/player_stub.cpp	2014-07-22 15:03:36 +0000
> +++ src/core/media/player_stub.cpp	2014-09-03 04:19:37 +0000
> @@ -275,7 +275,7 @@
>      return op.value();
>  }
>  
> -bool media::PlayerStub::open_uri(const Track::UriType& uri, std::string& cookies,
> +bool media::PlayerStub::open_uri(const Track::UriType& uri, const std::string& cookies,
>          const std::string& user_agent)
>  {
>      auto op = d->object->invoke_method_synchronously<mpris::Player::OpenUriExtended, bool>(uri, cookies, user_agent);
> 
> === modified file 'tests/unit-tests/test-gstreamer-engine.cpp'
> --- tests/unit-tests/test-gstreamer-engine.cpp	2014-06-24 20:37:44 +0000
> +++ tests/unit-tests/test-gstreamer-engine.cpp	2014-09-03 04:19:37 +0000
> @@ -151,6 +151,31 @@
>                      std::chrono::seconds{10}));
>  }
>  
> +TEST(GStreamerEngine, setting_uri_and_audio_playback_with_http_headers_works)
> +{
> +    const std::string test_audio_uri{"http://stream-dc1.radioparadise.com/mp3-32"};
> +
> +    core::testing::WaitableStateTransition<core::ubuntu::media::Engine::State> wst(
> +                core::ubuntu::media::Engine::State::ready);
> +
> +    gstreamer::Engine engine;
> +
> +    engine.state().changed().connect(
> +                std::bind(
> +                    &core::testing::WaitableStateTransition<core::ubuntu::media::Engine::State>::trigger,
> +                    std::ref(wst),
> +                    std::placeholders::_1));
> +
> +    EXPECT_TRUE(engine.open_resource_for_uri(test_audio_uri, "", "MediaHub"));
> +    EXPECT_TRUE(engine.play());
> +    EXPECT_TRUE(wst.wait_for_state_for(
> +                    core::ubuntu::media::Engine::State::playing,
> +                    std::chrono::seconds{10}));
> +    EXPECT_TRUE(wst.wait_for_state_for(
> +                    core::ubuntu::media::Engine::State::ready,
> +                    std::chrono::seconds{10}));
> +}
> +
>  TEST(GStreamerEngine, stop_pause_play_seek_audio_only_works)
>  {
>      const std::string test_file{"/tmp/test.ogg"};
> 


-- 
https://code.launchpad.net/~justinmcp/media-hub/oxide-support/+merge/233152
Your team Ubuntu Phablet Team is subscribed to branch lp:~phablet-team/media-hub/oxide-support.



More information about the Ubuntu-reviews mailing list