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 796714 - v4l2: add min-buffers properties
v4l2: add min-buffers properties
Status: RESOLVED WONTFIX
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2018-06-28 19:14 UTC by Peter Seiderer
Modified: 2018-06-29 14:35 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH v1 1/2] v4l2transform: fold property set/get PROP_OUTPUT_IO_MODE case into default (1.33 KB, patch)
2018-06-28 19:14 UTC, Peter Seiderer
committed Details | Review
[PATCH v1 2/2] v4l2: add min-buffers properties (8.89 KB, patch)
2018-06-28 19:15 UTC, Peter Seiderer
none Details | Review

Description Peter Seiderer 2018-06-28 19:14:19 UTC
Created attachment 372866 [details] [review]
[PATCH v1 1/2] v4l2transform: fold property set/get PROP_OUTPUT_IO_MODE case into default

Add min-buffers (capture-min-buffers/output-min-buffers) properties to v4l2src,
v4l2sink, v4l2videodec, v4l2videoenc and v4l2transform.
Comment 1 Peter Seiderer 2018-06-28 19:15:25 UTC
Created attachment 372867 [details] [review]
[PATCH v1 2/2] v4l2: add min-buffers properties
Comment 2 Nicolas Dufresne (ndufresne) 2018-06-28 19:41:12 UTC
My usual answer is no for that feature. You may want to file a bug with the actual situation were you have buffer starvation, and only if that cannot be resolved by fixing the allocation query implementation then maybe, but that has never happen.
Comment 3 Nicolas Dufresne (ndufresne) 2018-06-28 20:41:13 UTC
If the use case is as described in:

https://bugzilla.gnome.org/show_bug.cgi?id=796717#c6

Then this should be on appsrc/appsink (even though it's a bit more work considering pool support is absent there.
Comment 4 Nicolas Dufresne (ndufresne) 2018-06-28 20:42:38 UTC
There is also this trick that can be used:

https://cgit.freedesktop.org/mesa/kmscube/tree/gst-decoder.c#n241
Comment 5 Nicolas Dufresne (ndufresne) 2018-06-28 20:51:06 UTC
Comment on attachment 372866 [details] [review]
[PATCH v1 1/2] v4l2transform: fold property set/get PROP_OUTPUT_IO_MODE case into default

Pushed fold property set/get ... as 8ab3b91a8baf9b9ee74ed525a06f1ae6f7223be6
Comment 6 Peter Seiderer 2018-06-29 12:15:52 UTC
(In reply to Nicolas Dufresne (ndufresne) from comment #2)
> My usual answer is no for that feature. You may want to file a bug with the
> actual situation were you have buffer starvation, and only if that cannot be
> resolved by fixing the allocation query implementation then maybe, but that
> has never happen.

Use case is to reduce frame drops while using gst-launch pipelines (on i.mx6
you need the following kernel patches [1], [2] to enable gstreamer frame drop
detection), e.g. (17 frame dropee in 1.3 seconds):


	$ GST_DEBUG=*:3 gst-launch-1.0 -v v4l2src device=/dev/v4l/by-path/platform-capture-subsystem-video-index4 io-mode=dmabuf ! \
        video/x-raw,format=YUY2,width=1920,height=1080,framerate=60000/1001 ! \
        kmssink connector-id=47
Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
/GstPipeline:pipeline0/GstKMSSink:kmssink0: display-width = 0
/GstPipeline:pipeline0/GstKMSSink:kmssink0: display-height = 0
Setting pipeline to PLAYING ...
New clock: GstSystemClock
/GstPipeline:pipeline0/GstV4l2Src:v4l2src0.GstPad:src: caps = video/x-raw, format=(string)YUY2, width=(int)1920, height=(int)1080, framerate=(fraction)60000/1001, colorimetry=(string)bt709, interlace-mode=(string)progressive
/GstPipeline:pipeline0/GstCapsFilter:capsfilter0.GstPad:src: caps = video/x-raw, format=(string)YUY2, width=(int)1920, height=(int)1080, framerate=(fraction)60000/1001, colorimetry=(string)bt709, interlace-mode=(string)progressive
/GstPipeline:pipeline0/GstKMSSink:kmssink0.GstPad:sink: caps = video/x-raw, format=(string)YUY2, width=(int)1920, height=(int)1080, framerate=(fraction)60000/1001, colorimetry=(string)bt709, interlace-mode=(string)progressive
/GstPipeline:pipeline0/GstCapsFilter:capsfilter0.GstPad:sink: caps = video/x-raw, format=(string)YUY2, width=(int)1920, height=(int)1080, framerate=(fraction)60000/1001, colorimetry=(string)bt709, interlace-mode=(string)progressive
0:00:01.230732333  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 2 - ts: 0:00:00.272967653
0:00:01.330239333  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.339703987
0:00:01.364204000  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 2 - ts: 0:00:00.406437653
0:00:01.430929000  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.473165987
0:00:01.497655333  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.539894653
0:00:01.564386000  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.606622987
0:00:01.631112333  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.673351987
0:00:01.697845666  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.740082653
0:00:01.764580333  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.806809653
0:00:01.831302333  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.873539987
0:00:01.898026333  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.940269320
0:00:01.964755000  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:01.006997653
0:00:02.031484667  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:01.073726653
0:00:02.098214333  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:01.140456653
0:00:02.164946333  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:01.207185986
0:00:02.231672667  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:01.273914653
0:00:02.298400667  4124  0x17dc380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:01.340644653
[...]


With e.g. min-buffers=5 frame drops are reduced (17 frame drops in 21 seconds):

	$ GST_DEBUG=*:3 gst-launch-1.0 -v v4l2src device=/dev/v4l/by-path/platform-capture-subsystem-video-index4 min-buffers=5 io-mode=dmabuf ! \
        video/x-raw,format=YUY2,width=1920,height=1080,framerate=60000/1001 ! \
        kmssink connector-id=47
Setting pipeline to PAUSED ...
Pipeline is live and does not need PREROLL ...
/GstPipeline:pipeline0/GstKMSSink:kmssink0: display-width = 0
/GstPipeline:pipeline0/GstKMSSink:kmssink0: display-height = 0
Setting pipeline to PLAYING ...
New clock: GstSystemClock
/GstPipeline:pipeline0/GstV4l2Src:v4l2src0.GstPad:src: caps = video/x-raw, format=(string)YUY2, width=(int)1920, height=(int)1080, framerate=(fraction)60000/1001, colorimetry=(string)bt709, interlace-mode=(string)progressive
/GstPipeline:pipeline0/GstCapsFilter:capsfilter0.GstPad:src: caps = video/x-raw, format=(string)YUY2, width=(int)1920, height=(int)1080, framerate=(fraction)60000/1001, colorimetry=(string)bt709, interlace-mode=(string)progressive
/GstPipeline:pipeline0/GstKMSSink:kmssink0.GstPad:sink: caps = video/x-raw, format=(string)YUY2, width=(int)1920, height=(int)1080, framerate=(fraction)60000/1001, colorimetry=(string)bt709, interlace-mode=(string)progressive
/GstPipeline:pipeline0/GstCapsFilter:capsfilter0.GstPad:sink: caps = video/x-raw, format=(string)YUY2, width=(int)1920, height=(int)1080, framerate=(fraction)60000/1001, colorimetry=(string)bt709, interlace-mode=(string)progressive
0:00:01.254071333  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:00.286977643
0:00:03.854201333  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:02.889443976
0:00:03.937533000  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:02.972848310
0:00:04.020857000  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:03.056261977
0:00:04.104186667  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:03.139665976
0:00:04.187511000  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:03.223086977
0:00:04.270874667  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:03.306574976
0:00:04.354201000  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:03.389895976
0:00:04.437528000  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:03.473351976
0:00:20.554175669  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:19.588418310
0:00:21.687521002  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:20.722823976
0:00:21.770856336  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:20.806232977
0:00:21.854190002  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:20.889637310
0:00:21.937511002  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:20.973050310
0:00:22.020899669  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:21.056469310
0:00:22.104184336  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:21.139869976
0:00:22.187528336  4131   0xa99380 WARN                 v4l2src gstv4l2src.c:968:gst_v4l2src_create:<v4l2src0> lost frames detected: count = 1 - ts: 0:00:21.223287309
[...]


[1] https://patchwork.linuxtv.org/patch/47946/
[2] https://patchwork.linuxtv.org/patch/47947/
Comment 7 Peter Seiderer 2018-06-29 12:23:04 UTC
(In reply to Nicolas Dufresne (ndufresne) from comment #3)
> If the use case is as described in:
> 
> https://bugzilla.gnome.org/show_bug.cgi?id=796717#c6
> 
> Then this should be on appsrc/appsink (even though it's a bit more work
> considering pool support is absent there.

Yes, this would be another use case of the min-buffers feature ;-)

Thanks for the pad probe hint, helpful suggestion for the programmatically
approach (but not applicable for the gst-launch use case)...

Thanks for pushing the fist patch...
Comment 8 Nicolas Dufresne (ndufresne) 2018-06-29 13:56:49 UTC
That has nothing to do with the number of buffers, it's a well know issue with kmssink implementation, and the legacy DRM API being used. When v4l2src pushes buffers to kmssink, it ends up blocking for a variable amount of time, which cause overflow in v4l2src fifo. And it's specially critical on drivers like IMX.6 one that is an atomic render and does a synchronous commit on drmModeSetPlane().

The way to go is to implement atomic render support into kmssink, and make the renderer asynchronous so you only sync when the next buffer comes in. Note that well known workaround exist, first one was provided to my by Philippe Zabel, it is to set force-modesetting=1 in order to ensure we use PageFlip. And second one is to add a queue of at least 1 buffer before kmssink (to simulate an async render). Only downside is that the queue won't request more buffers like it should, another issue, but with Gst master, the default number of requested buffers by v4l2src has been increase for this purpose.

ps. In general, for smooth playback, any frame drop should happen in the sink element, base on the buffer timestamp, not just the collision of buffers base on jitter effect. I'll close this bug, as I have always done, because I prefer upstream fixes for real issues then ugly workaround. Feel free to keep that workaround in your vendor branch if you don't have the time.
Comment 9 Nicolas Dufresne (ndufresne) 2018-06-29 14:35:11 UTC
Useful reference:

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