After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 797066 - player: position getter returns 0 upon query failure
player: position getter returns 0 upon query failure
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.14.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-09-01 10:23 UTC by Philippe Normand
Modified: 2018-09-03 12:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (1.06 KB, patch)
2018-09-01 10:25 UTC, Philippe Normand
committed Details | Review
gstutils patch (2.40 KB, patch)
2018-09-03 11:11 UTC, Philippe Normand
accepted-commit_now Details | Review

Description Philippe Normand 2018-09-01 10:23:15 UTC
Returning 0 instead of GST_CLOCK_TIME_NONE in case of query failure is wrong because it might induce unintended seeks to the beginning of the media.

Arguably this might be an issue in gst_element_query_position() which doesn't set the output param to GST_CLOCK_TIME_NONE when the query failed.
Comment 1 Philippe Normand 2018-09-01 10:25:42 UTC
Created attachment 373536 [details] [review]
patch
Comment 2 Sebastian Dröge (slomo) 2018-09-03 07:29:04 UTC
Comment on attachment 373536 [details] [review]
patch

Same for the duration (please in the same commit), and while we're at it please also the corresponding change for gst_element_query_position()/duration() so that it always sets the value :)

Thanks for noticing!
Comment 3 Philippe Normand 2018-09-03 11:10:57 UTC
The duration query code in the player was already good, modulo one call site :) Will push the patch now and attach the one for gstutils.
Comment 4 Philippe Normand 2018-09-03 11:11:19 UTC
Created attachment 373539 [details] [review]
gstutils patch
Comment 5 Philippe Normand 2018-09-03 11:12:58 UTC
Comment on attachment 373536 [details] [review]
patch

commit d1ed94491ee2c4cda59b55978d446e518a8b516e (HEAD -> master, origin/master)
Author: Philippe Normand <philn@igalia.com>
Date:   Sat Sep 1 11:23:33 2018 +0100

    player: Set default position and duration value to GST_CLOCK_TIME_NONE
    
    When the position query fails the returned value shall remain -1 instead of 0 to
    avoid confusion on application side between error and beginning of media.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797066
Comment 6 Philippe Normand 2018-09-03 12:04:14 UTC
Cherry-picked to 1.14 as well:

commit d1b451f3e7b7033e0a841b7f2ebffe52c015ec3f (HEAD -> 1.14, origin/1.14)
Author: Philippe Normand <philn@igalia.com>
Date:   Sat Sep 1 11:23:33 2018 +0100

    player: Set default position and duration value to GST_CLOCK_TIME_NONE
    
    When the position query fails the returned value shall remain -1 instead of 0 to
    avoid confusion on application side between error and beginning of media.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797066
Comment 7 Philippe Normand 2018-09-03 12:28:16 UTC
Comment on attachment 373539 [details] [review]
gstutils patch

Thanks! Pushed in master and 1.14

commit 3cf1bac0b7ec6bb958d104392886e790990a8dca (HEAD -> 1.14, origin/1.14)
Author: Philippe Normand <philn@igalia.com>
Date:   Mon Sep 3 12:06:35 2018 +0100

    utils: Set default values for position and duration query results
    
    https://bugzilla.gnome.org/show_bug.cgi?id=797066