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 785680 - v4l2: Flags and Timestamp are lost when importing downstream buffer
v4l2: Flags and Timestamp are lost when importing downstream buffer
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-08-01 11:36 UTC by Merlin Manthey
Modified: 2017-08-09 13:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
proposed fix (1.14 KB, patch)
2017-08-01 11:36 UTC, Merlin Manthey
none Details | Review
v4l2bufferpool: Copy flags and timestamp when importing (1.75 KB, patch)
2017-08-01 13:24 UTC, Nicolas Dufresne (ndufresne)
none Details | Review
v4l2bufferpool: Copy flags and timestamp when importing (2.08 KB, patch)
2017-08-01 13:49 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
tee: Implement allocation query aggregation (7.44 KB, patch)
2017-08-09 13:46 UTC, Nicolas Dufresne (ndufresne)
none Details | Review

Description Merlin Manthey 2017-08-01 11:36:05 UTC
Created attachment 356715 [details] [review]
proposed fix

Hi,

i'm developing a v4l2 camera driver and a propriatary GstPlugin for custom post-processing. Our driver does sometimes fail to acquire images (for a known reason) and therefore the current v4l2_buffer flag V4L2_BUF_FLAG_ERROR is set. 

When using the v4l2src with no specific io-mode (I think GST_V4L2_IO_AUTO is used) the behavior is as expected. The GstBuffer is receive in my plugin has the flag GST_BUFFER_FLAG_CORRUPTED set whenever the V4L2_BUF_FLAG_ERROR flag is set.

If i use the io-mode GST_V4l2_IO_USERPTR i never receive the error flag.


I propose to fix this error with the attached patch. I'm not sure if its a good idea to do it the way i propose. Maybe it would be smarter to transfer only the error flag ?
Whats your opinion on this ?

Kind regards,
Merlin
Comment 1 Nicolas Dufresne (ndufresne) 2017-08-01 12:32:11 UTC
This looks like the right direction, though we should copy all the metadata. The is a flag in gst_buffer_copy to only copy that information. I'm surprised we don't have that already.
Comment 2 Nicolas Dufresne (ndufresne) 2017-08-01 13:24:32 UTC
Created attachment 356726 [details] [review]
v4l2bufferpool: Copy flags and timestamp when importing

Whenever we import from downstream pool (userptr or dmabuf-import), we
should copy over the flags and timestamp, otherwise downstream will not
get proper synchronization or will not be able to notice frames that has
corruption in it.
Comment 3 Nicolas Dufresne (ndufresne) 2017-08-01 13:28:16 UTC
Please, let me know if this patch also works as expected. This should copy the timestamps along with the flags.
Comment 4 Nicolas Dufresne (ndufresne) 2017-08-01 13:31:13 UTC
Review of attachment 356726 [details] [review]:

Oops, need to look into an issue with this one:
GStreamer-CRITICAL **: gst_buffer_copy_into: assertion 'gst_buffer_is_writable (dest)' failed
Comment 5 Nicolas Dufresne (ndufresne) 2017-08-01 13:49:23 UTC
Created attachment 356727 [details] [review]
v4l2bufferpool: Copy flags and timestamp when importing

[Update] Fixed the ref-counting so the buffer is writable upon copy

Whenever we import from downstream pool (userptr or dmabuf-import), we
should copy over the flags and timestamp, otherwise downstream will not
get proper synchronization or will not be able to notice frames that has
corruption in it.
Comment 6 Nicolas Dufresne (ndufresne) 2017-08-01 19:07:32 UTC
Ok, let 's give this a try. On Linux we need _GNU_SOURCE to be defined,
and on BSD, if I read the header properly, mmap64 is always there. It' s also
implemented in uClibc, and v4l2 is not built on Android / bionic, since we 
don 't have permissions to access the define from an Android app anyway.


Attachment 356727 [details] pushed as ba8aa44 - v4l2bufferpool: Copy flags and timestamp when importing
Comment 7 Nicolas Dufresne (ndufresne) 2017-08-01 19:09:02 UTC
Wrong bug, sorry for the confusion. I just tested this fix and it does fix the timestamps and flags properly.
Comment 8 Merlin Manthey 2017-08-02 07:56:29 UTC
Hi Nicolas, 

your fix works like a charm! 
Thank you very much for the fast response.

Kind regards,
Merlin
Comment 9 Nicolas Dufresne (ndufresne) 2017-08-09 13:46:25 UTC
Created attachment 357264 [details] [review]
tee: Implement allocation query aggregation

This will aggregate allocation params, pool and will keep all
meta that has no parameters.
Comment 10 Nicolas Dufresne (ndufresne) 2017-08-09 13:47:12 UTC
Comment on attachment 357264 [details] [review]
tee: Implement allocation query aggregation

Wrong bug.