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 766537 - identity: add property keep every N-th buffer
identity: add property keep every N-th buffer
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-05-16 23:58 UTC by Gregoire
Modified: 2018-11-03 12:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch identity drop every N frame (2.85 KB, patch)
2016-05-16 23:58 UTC, Gregoire
needs-work Details | Review
Keep every n-th buffer (2.79 KB, patch)
2016-05-17 17:06 UTC, Gregoire
reviewed Details | Review

Description Gregoire 2016-05-16 23:58:39 UTC
Created attachment 328029 [details] [review]
Patch identity drop every N frame

Here is a patch against head to add a property to the identity plugin in order to drop every N frames.

It seems to be a no-risk patch and add an handy feature if one wants to drop every other frame for instance.
Comment 1 Tim-Philipp Müller 2016-05-17 04:09:40 UTC
What's your use case here? You can't do what you want with videorate?
Comment 2 Gregoire 2016-05-17 16:18:53 UTC
The new title is not correct. It's more "keep every n-th frame" and discard the n-1 frames in-between.

Because of some intensive processing in the gstreamer application, I need to keep very precisely 1 out of n frames, hence the property.
Comment 3 Nicolas Dufresne (ndufresne) 2016-05-17 16:54:07 UTC
Identity does not deal with frames but buffers. A buffer can be a part of a frame, or multiple frame if the media type has the notion of frame. Tim question is relevent, would be nice to get an answer. Also, make sure newly create API does not use the term frame here.
Comment 4 Nicolas Dufresne (ndufresne) 2016-05-17 16:55:46 UTC
Review of attachment 328029 [details] [review]:

::: plugins/elements/gstidentity.c
@@ +213,3 @@
           G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS));
+  g_object_class_install_property (gobject_class, PROP_DROP_EVERY_N,
+      g_param_spec_int ("drop-every-n", "Drop every N frames",

identity does not have the notion of frame. Also, your comment contradict the name and the description of the property.
Comment 5 Gregoire 2016-05-17 17:02:00 UTC
Thanks for the feed-back. Fair enough.

Actually, your point makes it more clear that such property can do things that videorate can't do, aka. keeping every n-th buffer.

I'm not sure which question of Tim I haven't answered.

I will rework the patch by changing the name to keep-every-n, "Keep every n-th buffer".
Comment 6 Gregoire 2016-05-17 17:06:06 UTC
Created attachment 328093 [details] [review]
Keep every n-th buffer
Comment 7 Nicolas Dufresne (ndufresne) 2016-05-17 17:11:14 UTC
I might have answered for you indeed. Basically, if this is video related, you can already keep 1 out of N frames (precisely) using videorate. Let's say you have a 30/1 fps. You'd add a downstream filter at 3/1 fps. Videorate will effectively be keeping 3 frames every second, dropping evenly other frames. So Tim question rephrased is, is this video related ? If so, why don't you use videorate instead ?
Comment 8 Gregoire 2016-05-17 17:15:32 UTC
Because I don't know the framerate of the video and it changes slightly over different video. My specific use case is to have a precise drop n-1 frame. I might use videorate but I would have do some calculation and I'm not sure if it will match exactly my need of keep every n-th frame consistently.

As you said, you add a better argument for me! Perhaps some people might have a use case to drop BUFFER.
Comment 9 Nicolas Dufresne (ndufresne) 2016-05-17 17:24:25 UTC
Comment on attachment 328029 [details] [review]
Patch identity drop every N frame

Fyi, there is a obsolete checkbox to hide the old patches.
Comment 10 Nicolas Dufresne (ndufresne) 2016-05-17 17:34:08 UTC
Review of attachment 328093 [details] [review]:

I'm personally fine with this feature. My only remaining concern is flexibility. This implementation keep the last of N buffers (an enhancement would be to document this aspect). I can foresee someone that would like to keep the first of N instead. Would it be over-engineering to ask for a way to choose which of Nth buffers to drop ? This could be done with a second property, or by overloading the meaning of GstFraction (is that too ugly ?) ?
Comment 11 Tim-Philipp Müller 2016-05-17 17:42:58 UTC
It feels like a very specific thing to me. Do we really need to add a property to identity for that? It can easily be achieved with a pad probe too.
Comment 12 Gregoire 2016-05-17 17:45:50 UTC
I could add the second property but isn't it becoming a gas factory? ;-)

In my use case, I wasn't able to implement it in a pad probe though I tried very hard. My identity was between decodebin and glimagesink and with hardware acceleration on Android where the buffer is passed directly from the decoder to the GPU, returning drop flow in the pad was not working.
Comment 13 Nicolas Dufresne (ndufresne) 2016-05-17 18:03:00 UTC
(In reply to Gregoire from comment #12)
> I could add the second property but isn't it becoming a gas factory? ;-)
> 
> In my use case, I wasn't able to implement it in a pad probe though I tried
> very hard. My identity was between decodebin and glimagesink and with
> hardware acceleration on Android where the buffer is passed directly from
> the decoder to the GPU, returning drop flow in the pad was not working.

A pad probe returning GST_PAD_PROBE_DROP should definitely work if done correctly.

(In reply to Tim-Philipp Müller from comment #11)
> It feels like a very specific thing to me. Do we really need to add a
> property to identity for that? It can easily be achieved with a pad probe
> too.

My argument would be that all debugging property in identity were added for specific use cases initially.
Comment 14 Tim-Philipp Müller 2016-05-17 18:39:24 UTC
> My argument would be that all debugging property in identity
> were added for specific use cases initially.

Of course, I just don't find this one useful enough in general to warrant putting it into identity. If you do, go for it :)

But if we start adding more properties to cover the behaviour of the n-drop/keep, perhaps that's an indication that it should go into a separate element.
Comment 15 GStreamer system administrator 2018-11-03 12:34:41 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gstreamer/issues/172.