[Merge] lp:~justinmcp/media-hub/1603729 into lp:media-hub

Santosh santosh.mahto at canonical.com
Wed Aug 17 11:33:18 UTC 2016


I have just looked into code from coding prespective not other aspects. So I have one comment inline.
Its ok from my side if that is correct.



Diff comments:

> 
> === modified file 'src/core/media/player_implementation.cpp'
> --- src/core/media/player_implementation.cpp	2016-07-11 01:21:38 +0000
> +++ src/core/media/player_implementation.cpp	2016-08-17 07:37:18 +0000
> @@ -853,28 +873,13 @@
>  template<typename Parent>
>  bool media::PlayerImplementation<Parent>::open_uri(const Track::UriType& uri)
>  {
> -    d->track_list->reset();
> -
> -    // If empty uri, give the same meaning as QMediaPlayer::setMedia("")
> -    if (uri.empty())
> -    {
> -        MH_DEBUG("Resetting current media");
> -        return true;
> -    }
> -
> -    static const bool do_pipeline_reset = false;
> -    const bool ret = d->engine->open_resource_for_uri(uri, do_pipeline_reset);
> -    // Don't set new track as the current track to play since we're calling open_resource_for_uri above
> -    static const bool make_current = false;
> -    d->track_list->add_track_with_uri_at(uri, media::TrackList::after_empty_track(), make_current);
> -
> -    return ret;
> +    return d->open_uri(uri);
>  }
>  
>  template<typename Parent>
>  bool media::PlayerImplementation<Parent>::open_uri(const Track::UriType& uri, const Player::HeadersType& headers)
>  {
> -    return d->engine->open_resource_for_uri(uri, headers);
> +    return d->open_uri(uri, headers);

I have doubt here if we should  use open_resource_for_uri instead of open_uri

>  }
>  
>  template<typename Parent>


-- 
https://code.launchpad.net/~justinmcp/media-hub/1603729/+merge/303097
Your team Ubuntu Phablet Team is requested to review the proposed merge of lp:~justinmcp/media-hub/1603729 into lp:media-hub.



More information about the Ubuntu-reviews mailing list