[Merge] lp:~jhodapp/mediaplayer-app/mh3-fix-manual-eos into lp:mediaplayer-app

[Ricardo Mendoza]

Review: Needs Fixing

Some minor comments.

Diff comments:

> === modified file 'src/qml/player/Controls.qml'
> --- src/qml/player/Controls.qml	2014-10-10 15:37:02 +0000
> +++ src/qml/player/Controls.qml	2015-03-10 03:06:35 +0000
> @@ -19,6 +19,7 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  import QtQuick 2.0
> +import QtMultimedia 5.0
>  import Ubuntu.Components 1.1
>  
>  Item {
> @@ -30,6 +31,7 @@
>      property alias sceneSelectorHeight: _sceneSelector.height
>      property alias sceneSelectorVisible: _sceneSelector.visible
>      property int heightOffset: 0
> +    property variant playerStatus: MediaPlayer.NoMedia
>  
>      property alias settingsEnabled: _settingsButton.enabled
>  
> @@ -328,6 +330,14 @@
>          }
>      }
>  
> +    Connections {
> +        target: controls
> +        onPlayerStatusChanged: {
> +            console.debug("onPlayerStatusChanged")
> +            _timeline.playerStatus = controls.playerStatus
> +        }
> +    }
> +
>      states: [
>          State {
>              name: "stopped"
> 
> === modified file 'src/qml/player/TimeLine.qml'
> --- src/qml/player/TimeLine.qml	2014-10-09 20:28:57 +0000
> +++ src/qml/player/TimeLine.qml	2015-03-10 03:06:35 +0000
> @@ -18,6 +18,7 @@
>   */
>  
>  import QtQuick 2.0
> +import QtMultimedia 5.0
>  import Ubuntu.Components 1.1
>  import "../sdk"
>  
> @@ -27,6 +28,7 @@
>  
>      readonly property alias liveValue: _slider.value
>      property real videoPosition: -1
> +    property variant playerStatus: MediaPlayer.NoMedia
>      property string currentTime
>      property string remainingTime
>  
> @@ -57,7 +59,15 @@
>      maximumValue: 1000
>      live: true
>      onVideoPositionChanged: {
> -        if (!_slider.pressed) {
> +        if (_slider.playerStatus == MediaPlayer.EndOfMedia)
> +        {
> +            // On EndOfMedia status, make sure the slider returns to the beginning
> +            _slider.value = 0

I dont agree with this one, it should let it update from _slider.videoPosition if its EndOfMedia, otherwise we might be letting through a bug and hiding it in the UI (position not really resetting to 0, but the slider going to 0).

> +        } else if (pressed && _slider.videoPosition == 0) {

Should probably cork these unwanted events at media-hub, PlayerImpl, otherwise same issue as above, hiding the bug in the client app means another one will see the same.

> +            // Ignore when GStreamer returns a 0 position while seeking
> +            return
> +        } else {
> +            // Else, pass all new positions through to the slider UI
>              _slider.value = _slider.videoPosition
>          }
>      }
> 
> === modified file 'src/qml/player/VideoPlayer.qml'
> --- src/qml/player/VideoPlayer.qml	2015-02-13 02:49:27 +0000
> +++ src/qml/player/VideoPlayer.qml	2015-03-10 03:06:35 +0000
> @@ -108,6 +108,7 @@
>  
>              maximumHeight: units.gu(27)
>              sceneSelectorHeight: units.gu(18)
> +            playerStatus: player.status
>  
>              onPlaybackClicked: player.playPause()
>  
> @@ -129,7 +130,9 @@
>              }
>  
>              onEndSeek: {
> -                if (!isPaused) {
> +                // Only automatically resume playing after a seek that is not to the
> +                // end of stream (i.e. position == duration)
> +                if (player.status != MediaPlayer.EndOfMedia && !isPaused) {

I wonder why they have this pause-on-seek logic, it seems kind of a waste as all they do is play again afterwards. The spec doesn't pause on seek by default.

>                      player.play()
>                  }
>              }
> 


-- 
https://code.launchpad.net/~jhodapp/mediaplayer-app/mh3-fix-manual-eos/+merge/252396
Your team Ubuntu Phablet Team is subscribed to branch lp:mediaplayer-app.