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

2019-04-06 Thread Carl Eugen Hoyos 
2018-12-09 18:50 GMT+01:00, Mark Thompson :
> 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 
>> 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.

OpenBSD also uses the glibc-variant of ioctl().
How should this be fixed?

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

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

2018-12-10 Thread Carl Eugen Hoyos 
2018-12-10 23:47 GMT+01:00, Mark Thompson :
> On 10/12/2018 00:47, Carl Eugen Hoyos wrote:
>> 2018-12-09 18:50 GMT+01:00, Mark Thompson :
>>> 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 
 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__
>  #else  #endif", rather than fixing it
> only for Android as you have here.

Applied with your suggestion, thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-12-10 Thread Mark Thompson 
On 10/12/2018 00:47, Carl Eugen Hoyos wrote:
> 2018-12-09 18:50 GMT+01:00, Mark Thompson :
>> 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 
>>> 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.  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__ 
 #else  #endif", rather than fixing it only 
for Android as you have here.

Thanks,

- Mark
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-12-09 Thread Carl Eugen Hoyos 
2018-12-09 18:50 GMT+01:00, Mark Thompson :
> 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 
>> 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?

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-12-09 Thread Mark Thompson 
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 
> 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.

Thanks,

- Mark


* POSIX - 
.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

2018-12-06 Thread Carl Eugen Hoyos 
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 
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

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel