GNOME Bugzilla – Bug 768301
buffer: add explicit setters and getters for GstBufferFlags
Last modified: 2016-08-23 07:51:53 UTC
Created attachment 330754 [details] [review] Proposed patch GST_BUFFER_FLAGS, GST_BUFFER_FLAG_IS_SET, GST_BUFFER_FLAG_SET and GST_BUFFER_FLAG_UNSET do not have an explicit method that gi-introspection can parse. Therefore, these macros cannot be used from Python. In contrast, the GstBufferFlags are already exposed under Gst.BufferFlags. This patch adds the necessary setters and getters. (Gst.Buffer.flags, Gst.Buffer.flag_is_set, Gst.Buffer.flag_set and Gst.Buffer.flag_unset respectively).
Comment on attachment 330754 [details] [review] Proposed patch Thanks for the patch, I think this makes sense. Couple of comments: - (transfer none) doesn't make sense for booleans and integer types - please add "Since: 1.10" markers at the end of each gtk-doc blob - not quite sure about naming, in particular: - gst_buffer_flags() -> gst_buffer_get_flags() ? - gst_buffer_flag_set() -> gst_buffer_set_flags() ? [can set multiple in one go] - gst_buffer_flag_unset() -> gst_buffer_unset_flags() ? [ditto] - gst_buffer_flag_is_set() -> gst_buffer_check_flags() _has_flags() is_flag_set? [not sure]
Hi Tim, Thanks for your feedback! - (transfer none) doesn't make sense for booleans and integer types --> fixed - please add "Since: 1.10" markers at the end of each gtk-doc blob --> added - not quite sure about naming, in particular: - gst_buffer_flags() -> gst_buffer_get_flags() ? --> changed - gst_buffer_flag_set() -> gst_buffer_set_flags() ? [can set multiple in one go] --> changed - gst_buffer_flag_unset() -> gst_buffer_unset_flags() ? [ditto] --> changed - gst_buffer_flag_is_set() -> gst_buffer_check_flags() _has_flags() is_flag_set? [not sure] --> changed At first I tried to keep the method names in sync with the macro names, but I agree that your proposed method names are more clear. I will upload a new patch in a couple of minutes.
Created attachment 330823 [details] [review] Revised patch after first review by Tim
Created attachment 330826 [details] [review] Revised patch (v3) Fixed typo: gst_buffer_flag_has_flags -> gst_buffer_has_flags
Thanks for the patch, pushed with minor mostly cosmetic changes: commit cec16310458b1fe3b1e0ee42c27c17cbc6f4a400 Author: Arjen Veenhuizen <arjen.veenhuizen@tno.nl> Date: Thu Jul 7 08:01:24 2016 +0200 buffer: add explicit getters and setters for buffer flags https://bugzilla.gnome.org/show_bug.cgi?id=768301 Would be great if you could provide the patch in git format-patch format next time (ie with author and commit message included) :)
(In reply to Tim-Philipp Müller from comment #5) > Thanks for the patch, pushed with minor mostly cosmetic changes: > > commit cec16310458b1fe3b1e0ee42c27c17cbc6f4a400 > Author: Arjen Veenhuizen <arjen.veenhuizen@tno.nl> > Date: Thu Jul 7 08:01:24 2016 +0200 > > buffer: add explicit getters and setters for buffer flags > > https://bugzilla.gnome.org/show_bug.cgi?id=768301 > > > Would be great if you could provide the patch in git format-patch format > next time (ie with author and commit message included) :) I will do that next time! Great that this found its way upstream!