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 791779 - v4l2src: mmap64 is not available on FreeBSD
v4l2src: mmap64 is not available on FreeBSD
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other FreeBSD
: Normal normal
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-12-19 10:48 UTC by Ting-Wei Lan
Modified: 2018-02-18 21:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
v4l2object: Check for mmap64 before using it (1.64 KB, patch)
2017-12-19 10:49 UTC, Ting-Wei Lan
none Details | Review
v4l2object: Don't use mmap64 if off_t is 64-bit (1.92 KB, patch)
2017-12-22 03:45 UTC, Ting-Wei Lan
committed Details | Review

Description Ting-Wei Lan 2017-12-19 10:48:49 UTC
Commit b61bba4 makes sys/v4l2/gstv4l2object.c use mmap64 unconditionally, but mmap64 is not specified in the POSIX standard and it is not available on some systems. We have to check for it before using it to keep gst-plugins-good work on FreeBSD.
Comment 1 Ting-Wei Lan 2017-12-19 10:49:26 UTC
Created attachment 365739 [details] [review]
v4l2object: Check for mmap64 before using it

mmap64 is not available on FreeBSD.
Comment 2 Tim-Philipp Müller 2017-12-19 11:05:52 UTC
Should mmap64 be used directly? Shouldn't we rather use mmap and set some flags so that it maps to 64-bit sizes?
Comment 3 Nicolas Dufresne (ndufresne) 2017-12-19 13:03:34 UTC
Review of attachment 365739 [details] [review]:

I think for freebsd, We will need a wrapper to check the overflow. Also you are assigning to an incompatible function pointer. Its strange since I thought it was supported by other flavour of bsd, maybe I missed checked. Means you cannot mmap large files on FreeBSD?
Comment 4 Nicolas Dufresne (ndufresne) 2017-12-19 13:06:30 UTC
(In reply to Tim-Philipp Müller from comment #2)
> Should mmap64 be used directly? Shouldn't we rather use mmap and set some
> flags so that it maps to 64-bit sizes?

The function signature are from libv4l2. We should always prefer 64 bit variant in newly written code imho.
Comment 5 Nicolas Dufresne (ndufresne) 2017-12-19 13:11:19 UTC
I'm not sure that mmap64 not being part of POSIX is right.
Comment 6 Ting-Wei Lan 2017-12-19 13:13:57 UTC
mmap on FreeBSD is declared in sys/mman.h as:

void *  mmap(void *, size_t, int, int, int, off_t);

and off_t is defined in sys/_types.h as int64_t.


off_t is not defined in a machine-dependent header file, so I think it is always 64-bit regardless of architecture.
Comment 7 Nicolas Dufresne (ndufresne) 2017-12-19 18:45:19 UTC
Ok, so FreeBSD is hardcoding it to be equivalent of mmap64 on Linux, regardless of if you are running on 32bit or not. Can you then provide a cleaner patch, something that makes this clear and FreeBSD or better sizeof(off_t) == 8 specific ? Something that ends up doing:

#ifdef ...
#define mmap64 mmap
#endif
Comment 8 Nicolas Dufresne (ndufresne) 2017-12-19 18:48:53 UTC
Review of attachment 365739 [details] [review]:

::: sys/v4l2/gstv4l2object.c
@@ +519,3 @@
     v4l2object->mmap = mmap64;
+#else
+    v4l2object->mmap = mmap;

To explain better, here you assume that not having mmap64 implies that off_t == int64_t . Check for the exact thing you are looking for, e.g. that sizeof(off_t) == 8. The signature I'm trying to match is the following:

LIBV4L_PUBLIC void *v4l2_mmap(void *start, size_t length, int prot, int flags, int fd, int64_t offset);
Comment 9 Nicolas Dufresne (ndufresne) 2017-12-22 02:55:39 UTC
Ok, both OpenBSD and FreeBSD matches libv4l2 signature of v4l2_mmap. Let's assume if a plaform defines mmap64, then it shall be used. So I agree with your patch.
Comment 10 Nicolas Dufresne (ndufresne) 2017-12-22 02:56:09 UTC
Review of attachment 365739 [details] [review]:

Let's merge this.
Comment 11 Nicolas Dufresne (ndufresne) 2017-12-22 02:59:05 UTC
Thanks, and sorry for the confusion.

Attachment 365739 [details] pushed as 7843482 - v4l2object: Check for mmap64 before using it
Comment 12 Ting-Wei Lan 2017-12-22 03:45:03 UTC
Created attachment 365868 [details] [review]
v4l2object: Don't use mmap64 if off_t is 64-bit

The difference between mmap and mmap64 is the type of 'offset' argument.
mmap64 always uses a 64-bit interger as offset, while mmap uses off_t,
whose size can vary on different operating systems or architectures.

However, not all operating systems support mmap64. Fortunately, although
FreeBSD only has mmap, its off_t is always 64-bit regardless of
architectures, so we can simply use mmap when sizeof(off_t) == 8.
Comment 13 Ting-Wei Lan 2017-12-22 03:55:08 UTC
Sorry, I didn't check the status of the patch before uploading a new patch ...
Comment 14 Nicolas Dufresne (ndufresne) 2017-12-22 04:40:39 UTC
Review of attachment 365868 [details] [review]:

That patch looks better indeed, ok, I'll probably replace tomorrow then, thank you.
Comment 15 Nicolas Dufresne (ndufresne) 2017-12-22 15:38:16 UTC
Attachment 365868 [details] pushed as 4026211 - v4l2object: Don't use mmap64 if off_t is 64-bit
Comment 16 Nicolas Dufresne (ndufresne) 2017-12-22 15:39:05 UTC
I've reverted the previous one and replaced by this one. Thanks a lot of your work.
Comment 17 Elnur Ismailzada 2018-02-18 21:09:04 UTC
I have problem with mmap on Raspberry Pi 3.

gstv4l2object.c: In function ‘gst_v4l2_object_new’:
gstv4l2object.c:520:22: error: assignment from incompatible pointer type [-Werror=incompatible-pointer-types]
     v4l2object->mmap = mmap;
                      ^
cc1: all warnings being treated as errors

What could be the problem?
Comment 18 Nicolas Dufresne (ndufresne) 2018-02-18 21:27:33 UTC
Maybe you could comment on https://bugzilla.gnome.org/show_bug.cgi?id=793103 and help fixing this.