Created attachment 338485 [details] [review]
patch for add accurate seek

The reason why the set_seek_accurate API use GstPlayer *player as parameter instead of * config, is that if using config, we have get_config() first and then set_config() back, however for set_config(), only in STOPPED status it can set successfully, so if we want to seek when playing (either accurate or no-accurate), we have to call gst_player_stop() first, then set_config(). but it takes time to change state to STOPPED which might cause set_config failed. 

So here use GstPlayer *player as input parameter, so that we can set accurate flag in player->config directly without have to stop the pipeline.

Please kindly find the attached file for the patch.

Thanks
Lyon

Review of attachment 338485 [details] [review]:

Thanks, looks good except for cosmetic things and docs

::: gst-libs/gst/player/gstplayer.c
@@ +4219,3 @@
+ *
+ * set accurate seek flag.
+ */

Since: 1.12 (also for the other function)


Please also explain in more detail in the documentation what this actually means

@@ +4235,3 @@
+ * @config: a #GstPlayer configuration
+ *
+ * Returns: current position update interval in milliseconds

Copy and paste error from other documentation

@@ +4242,3 @@
+gst_player_config_get_seek_accurate(const GstStructure * config)
+{
+  gboolean  accurate = FALSE;

Double space before the variable name here. please run the .c files through gst-indent

::: gst-libs/gst/player/gstplayer.h
@@ +205,3 @@
+void           gst_player_config_set_seek_accurate  (GstPlayer *player,
+                                                                gboolean          accurate);
+gboolean       gst_player_config_get_seek_accurate  (const GstStructure * config);

Fix up indentation please

Created attachment 338494 [details] [review]
modified patch for add accurate seek

Modified patch according to the review comments

Review of attachment 338494 [details] [review]:

Please run your .c files through gst-indent


(If you can, update the patch, otherwise I'll fix up these things when merging after 1.10.0)

::: gst-libs/gst/player/gstplayer.c
@@ +4219,3 @@
+ *
+ * set accurate seek flag.
+ * If set, seek process will get to accurate seek position

Basically what this should say is something like:

Enable/disable accurate seeking. When enabled, if possible, elements will try harder to seek as accurately as possible to the requested seek position and generally seeking can be slower. Without this flag, they will seek as close to the requested seek position as is possible without slowing down seeking too much.



Maybe also taking a look at the wording of the GST_SEEK_FLAG_ACCURATE docs could help here.

::: gst-libs/gst/player/gstplayer.h
@@ +205,3 @@
+void           gst_player_config_set_seek_accurate  (GstPlayer *player,
+                                                     gboolean  accurate);
+gboolean       gst_player_config_get_seek_accurate  (const GstStructure * config);

And fix the indentation here to be aligned with the functions above :)

Created attachment 338570 [details] [review]
Update version according to slomo's comments

slomo, thanks so much for your kindly review: 
patch updated:
- gst-indent the gstplayer.c
- Updated documentation for set_seek_accurate API
- fix indentation in gstplayer.h

Created attachment 338571 [details] [review]
Update caccording to slomo's commnets

Sorry, previous patch is uploaded wrongly.
Please ignore previous one and use this one.

commit 8d41d816a240b0a4739e99640a382d0beab670e0
Author: Lyon Wang <lyon.wang@nxp.com>
Date:   Wed Oct 26 16:28:10 2016 +0800

    player: Add configuration for enabling accurate seeks
    
    https://bugzilla.gnome.org/show_bug.cgi?id=773521