[FFmpeg-devel] [PATCH]lavd/v4l2: Use ioctl(..., "int request" ) on Android

[Carl Eugen Hoyos]

2018-12-10 23:47 GMT+01:00, Mark Thompson <sw at jkqxz.net>:
> On 10/12/2018 00:47, Carl Eugen Hoyos wrote:
>> 2018-12-09 18:50 GMT+01:00, Mark Thompson <sw at jkqxz.net>:
>>> On 06/12/2018 22:37, Carl Eugen Hoyos wrote:
>>>> Hi!
>>>>
>>>> Attached patch fixes building with new Android toolchain, used to be a
>>>> warning.
>>>>
>>>> Please comment, Carl Eugen
>>>>
>>>> From d366c948af086520bfb2a4048e76f8d117690776 Mon Sep 17 00:00:00 2001
>>>> From: Carl Eugen Hoyos <ceffmpeg at gmail.com>
>>>> Date: Thu, 6 Dec 2018 23:34:54 +0100
>>>> Subject: [PATCH] lavd/v4l2: Use "int request" as second parameter for
>>>> ioctl()
>>>>  on Android.
>>>>
>>>> Fixes build with new Android toolchain.
>>>> ---
>>>>  libavdevice/v4l2.c |    4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/libavdevice/v4l2.c b/libavdevice/v4l2.c
>>>> index 10a0ff0..aa7c052 100644
>>>> --- a/libavdevice/v4l2.c
>>>> +++ b/libavdevice/v4l2.c
>>>> @@ -95,7 +95,11 @@ struct video_data {
>>>>      int (*open_f)(const char *file, int oflag, ...);
>>>>      int (*close_f)(int fd);
>>>>      int (*dup_f)(int fd);
>>>> +#ifdef __ANDROID__
>>>> +    int (*ioctl_f)(int fd, int request, ...);
>>>> +#else
>>>>      int (*ioctl_f)(int fd, unsigned long int request, ...);
>>>> +#endif
>>>>      ssize_t (*read_f)(int fd, void *buffer, size_t n);
>>>>      void *(*mmap_f)(void *start, size_t length, int prot, int flags,
>>>> int
>>>> fd, int64_t offset);
>>>>      int (*munmap_f)(void *_start, size_t length);
>>>> --
>>>> 1.7.10.4
>>>>
>>>
>>> LGTM on its own, but should this perhaps be "#ifndef (glibc something)"
>>> for
>>> the first case?  Looking at possible V4L2-hosting libcs, only glibc has
>>> the
>>> nonstandard* "unsigned long" rather than "int" as the request argument,
>>> so I
>>> expect we're going to hit this in more cases (e.g. musl) if compilers are
>>> now complaining about it.
>>
>> Is videodev2.h a Linux header as opposed to a c-library header or am I
>> completely misunderstanding?
>
> I'm not quite sure what you're asking, so I'll try to explain all of that
> more carefully:
>
> videodev2.h is a Linux header, but it only defines the types and values to
> call ioctl() with so is mostly orthogonal to this discussion.

I had not thought about this, you are of course right.

> ioctl() itself is a C library call, but since V4L2 is almost always used with glibc
> the prototype here has ended up matching that rather than the standard POSIX
> one.
>
> Android follows the POSIX prototype, so your patch is correct.  However, so
> do other standard C libraries such as Musl, and therefore I expect we will
> hit the same problem with other C libraries in the not-too-distant future.
> My suggestion, then, is to guard the change with "#ifdef __GLIBC__
> <glibc-prototype> #else <posix-prototype> #endif", rather than fixing it
> only for Android as you have here.

Applied with your suggestion, thank you, Carl Eugen