[Bug 773709] player: Add get video thumbnail API

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=773709

Sebastian Dröge (slomo) <slomo at coaxion.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #343349|none                        |needs-work
             status|                            |

--- Comment #16 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
Review of attachment 343349:
 --> (https://bugzilla.gnome.org/review?bug=773709&attachment=343349)

Typo in commit message: "sanpshot"

Please also include a link to this bug report, and the name of the new function
at least

::: gst-libs/gst/player/gstplayer.c
@@ +4264,3 @@
+ * gst_player_get_video_snapshot:
+ * @player: #GstPlayer instance
+ * @type: prefered output format

This is not just preferred, it *is* the output format. Also maybe call it
format, not type everywhere

@@ +4265,3 @@
+ * @player: #GstPlayer instance
+ * @type: prefered output format
+ * @config: Additional configuration including width/height/par-n/par-d

Just additional configuration. Also: "@config: (allow-none): ..."

@@ +4269,3 @@
+ * Get the video sample snapshot if video existed
+ # the sample includes data in current video and postion
+ # the prefered output format / width / height / pixel-aspect-ratio can be set

How about:

Get a snapshot of the currently selected video stream, if any. The format can
be selected with @type and optional additional configuration is possible with
@config. Currently supported settings are:
- width, height of type G_TYPE_INT
- pixel-aspect-ratio of type GST_TYPE_FRACTION

@@ +4271,3 @@
+ # the prefered output format / width / height / pixel-aspect-ratio can be set
+ *
+ * Returns: Current video snapshot sample

Returns: (transfer full): ... or %NULL on failure

@@ +4321,3 @@
+    if (!gst_structure_get_int(config, "par-n", &par_n))
+      par_n = 0;
+    if (!gst_structure_get_int(config, "par-d", &par_d))

Should be "pixel-aspect-ratio" of type GST_TYPE_FRACTION

@@ +4323,3 @@
+    if (!gst_structure_get_int(config, "par-d", &par_d))
+      par_d = 0;
+  }

If no config, except for native I would set pixel-aspect-ratio to 1/1 (and
mention that in the documentation).

::: gst-libs/gst/player/gstplayer.h
@@ +216,3 @@
+
+GstSample * gst_player_get_video_snapshot (GstPlayer * player,
+    GstPlayerThumbnailType type, GstStructure * config);

The structure could be const I guess

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.