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 795746 - gst-omx: should allocate the same number of buffers on consecutive ports when using dynamic buffer mode
gst-omx: should allocate the same number of buffers on consecutive ports when...
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-omx
unspecified
Other Linux
: Normal enhancement
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on: 791211 795747
Blocks:
 
 
Reported: 2018-05-02 09:02 UTC by Guillaume Desmottes
Modified: 2018-11-03 13:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
omxvideodec: implement propose_allocation (2.30 KB, patch)
2018-05-18 16:44 UTC, Guillaume Desmottes
none Details | Review
omxvideodec: implement propose_allocation (2.30 KB, patch)
2018-05-18 16:44 UTC, Guillaume Desmottes
committed Details | Review
omxvideoenc: implement decide_allocation (6.71 KB, patch)
2018-05-18 16:45 UTC, Guillaume Desmottes
committed Details | Review

Description Guillaume Desmottes 2018-05-02 09:02:21 UTC
On the Zynq, when using dynamic mode (including dma import), let's say we have "A ! B" with A's output port requiring N buffers and B's input port requiring M buffers. To prevent starvation, as buffers from A will be kept by B, we should ensure that both ports have N+M buffers allocated.
Comment 1 Guillaume Desmottes 2018-05-02 09:15:38 UTC
This would require extra API to notify downstream about allocated buffers: 795747
Comment 2 Guillaume Desmottes 2018-05-18 16:44:10 UTC
Created attachment 372189 [details] [review]
omxvideodec: implement propose_allocation

Tell upstream about how many buffer we plan to use so they can adjust
their own number of buffers accordingly if needed.

Same logic as the existing gst_omx_video_enc_propose_allocation().
Comment 3 Guillaume Desmottes 2018-05-18 16:44:58 UTC
Created attachment 372190 [details] [review]
omxvideodec: implement propose_allocation

Tell upstream about how many buffer we plan to use so they can adjust
their own number of buffers accordingly if needed.

Same logic as the existing gst_omx_video_enc_propose_allocation().
Comment 4 Guillaume Desmottes 2018-05-18 16:45:16 UTC
Created attachment 372191 [details] [review]
omxvideoenc: implement decide_allocation

Increase the number of output buffers by the number of buffers requested
downstream.
Prevent buffers starvation if downstream is going to use dynamic buffer
mode on its input.
Comment 5 Guillaume Desmottes 2018-05-18 16:47:01 UTC
Here is the first half of the fixes for this task. It doesn't require bug #795747  but is based on top of the patches from bug #791211
Comment 6 Nicolas Dufresne (ndufresne) 2018-06-07 19:11:54 UTC
It's not clear why this would cause starvation. Is that because of the side effect of the buffer stealing technique ? (e.g. placing foreign buffer into local pool ?).

Also, I really don't like the "extra" parameter.
Comment 7 Guillaume Desmottes 2018-06-08 07:55:01 UTC
Yes, in dynamic mode the element will keep upstream buffer alive until it has been processed by OMX which may starve the upstream pool.
Comment 8 Nicolas Dufresne (ndufresne) 2018-06-14 15:36:54 UTC
I realize my question was a bit stupid. I basically didn't understood the context, upstream / downstream. It makes a lot of sense.
Comment 9 Nicolas Dufresne (ndufresne) 2018-06-14 15:37:27 UTC
Review of attachment 372190 [details] [review]:

Lgtm.
Comment 10 Nicolas Dufresne (ndufresne) 2018-06-14 15:38:14 UTC
Review of attachment 372191 [details] [review]:

Lgtm.
Comment 11 Guillaume Desmottes 2018-06-15 07:58:14 UTC
Attachment 372190 [details] pushed as 431eac0 - omxvideodec: implement propose_allocation
Attachment 372191 [details] pushed as c8969b0 - omxvideoenc: implement decide_allocation
Comment 12 Guillaume Desmottes 2018-06-15 08:00:22 UTC
Thanks, I merged those patches.

Keeping this bug open as the second half requires bug #795747 to be addressed first.
Comment 13 GStreamer system administrator 2018-11-03 13:02:04 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/gst-omx/issues/22.