[FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Brad Smith]

Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
type should be an unsigned char on anything but Linux.


diff --git a/libavformat/udp.c b/libavformat/udp.c
index 180d96a988..29aa865fff 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 {
 #ifdef IP_MULTICAST_TTL
     if (addr->sa_family == AF_INET) {
-        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
+#ifdef __linux__
+        int ttl = mcastTTL;
+#else
+        unsigned char ttl = mcastTTL;
+#endif
+
+        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
             ff_log_net_error(NULL, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
             return ff_neterrno();
         }
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Brad Smith]

ping.

On 1/12/2022 12:13 AM, Brad Smith wrote:
> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
> type should be an unsigned char on anything but Linux.
>
>
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 180d96a988..29aa865fff 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>   {
>   #ifdef IP_MULTICAST_TTL
>       if (addr->sa_family == AF_INET) {
> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> +#ifdef __linux__
> +        int ttl = mcastTTL;
> +#else
> +        unsigned char ttl = mcastTTL;
> +#endif
> +
> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
>               ff_log_net_error(NULL, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>               return ff_neterrno();
>           }
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1007 bytes --]

On Wed, Jan 12, 2022 at 12:13:13AM -0500, Brad Smith wrote:
> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
> type should be an unsigned char on anything but Linux.
> 
> 
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 180d96a988..29aa865fff 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>  {
>  #ifdef IP_MULTICAST_TTL
>      if (addr->sa_family == AF_INET) {
> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> +#ifdef __linux__
> +        int ttl = mcastTTL;
> +#else
> +        unsigned char ttl = mcastTTL;
> +#endif

this "ifdef __linux__" feels like the wrong thing to check, dont you agree ?

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Marton Balint]

On Sun, 23 Jan 2022, Michael Niedermayer wrote:

> On Wed, Jan 12, 2022 at 12:13:13AM -0500, Brad Smith wrote:
>> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
>> type should be an unsigned char on anything but Linux.
>>
>>
>> diff --git a/libavformat/udp.c b/libavformat/udp.c
>> index 180d96a988..29aa865fff 100644
>> --- a/libavformat/udp.c
>> +++ b/libavformat/udp.c
>> @@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>>  {
>>  #ifdef IP_MULTICAST_TTL
>>      if (addr->sa_family == AF_INET) {
>> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
>> +#ifdef __linux__
>> +        int ttl = mcastTTL;
>> +#else
>> +        unsigned char ttl = mcastTTL;
>> +#endif
>
> this "ifdef __linux__" feels like the wrong thing to check, dont you agree ?

As far as I remember linux supports both sizes. So maybe just remove the 
check entirely?

Regards,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Brad Smith]

On 1/23/2022 6:57 AM, Michael Niedermayer wrote:

> On Wed, Jan 12, 2022 at 12:13:13AM -0500, Brad Smith wrote:
>> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
>> type should be an unsigned char on anything but Linux.
>>
>>
>> diff --git a/libavformat/udp.c b/libavformat/udp.c
>> index 180d96a988..29aa865fff 100644
>> --- a/libavformat/udp.c
>> +++ b/libavformat/udp.c
>> @@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>>   {
>>   #ifdef IP_MULTICAST_TTL
>>       if (addr->sa_family == AF_INET) {
>> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
>> +#ifdef __linux__
>> +        int ttl = mcastTTL;
>> +#else
>> +        unsigned char ttl = mcastTTL;
>> +#endif
> this "ifdef __linux__" feels like the wrong thing to check, dont you agree ?

Not sure what you mean.

But as I said in one of my other posts..

"FreeBSD, NetBSD, OpenBSD, DragonFlyBSD, macOS, Solaris, AIX, IRIX, 
HP-UX, QNX, Minix3 and a few
others define the ttl parameter to IP_MULTICAST_TTL as an unsigned char. 
Linux has it as an integer."

I looked for various examples of IP_MULTICAST_TTL usage in whatever 
projects I could find and most
of the examples I found used only unsigned char, with BIRD (routing 
daemon) being one of few that
use an int for Linux and unsigned char for *BSD's. It does not have 
support for any other OS's.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1485 bytes --]

On Sun, Jan 23, 2022 at 02:55:38PM -0500, Brad Smith wrote:
> On 1/23/2022 6:57 AM, Michael Niedermayer wrote:
> 
> > On Wed, Jan 12, 2022 at 12:13:13AM -0500, Brad Smith wrote:
> > > Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
> > > type should be an unsigned char on anything but Linux.
> > > 
> > > 
> > > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > > index 180d96a988..29aa865fff 100644
> > > --- a/libavformat/udp.c
> > > +++ b/libavformat/udp.c
> > > @@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> > >   {
> > >   #ifdef IP_MULTICAST_TTL
> > >       if (addr->sa_family == AF_INET) {
> > > -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > > +#ifdef __linux__
> > > +        int ttl = mcastTTL;
> > > +#else
> > > +        unsigned char ttl = mcastTTL;
> > > +#endif
> > this "ifdef __linux__" feels like the wrong thing to check, dont you agree ?
> 
> Not sure what you mean.

"__linux__" seems the wrong property to check for
this is not
#ifdef __linux__
osname = "linux"
#else

i would have expected more something like
#if HAVE_INT_TTL
int ttl = mcastTTL;
#else

or maybe even something along the lines of

WHATEVER_TTL_TYPE ttl = mcastTTL;

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Brad Smith]

On Mon, Jan 24, 2022 at 01:40:47PM +0100, Michael Niedermayer wrote:
> On Sun, Jan 23, 2022 at 02:55:38PM -0500, Brad Smith wrote:
> > On 1/23/2022 6:57 AM, Michael Niedermayer wrote:
> > 
> > > On Wed, Jan 12, 2022 at 12:13:13AM -0500, Brad Smith wrote:
> > > > Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
> > > > type should be an unsigned char on anything but Linux.
> > > > 
> > > > 
> > > > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > > > index 180d96a988..29aa865fff 100644
> > > > --- a/libavformat/udp.c
> > > > +++ b/libavformat/udp.c
> > > > @@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> > > >   {
> > > >   #ifdef IP_MULTICAST_TTL
> > > >       if (addr->sa_family == AF_INET) {
> > > > -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > > > +#ifdef __linux__
> > > > +        int ttl = mcastTTL;
> > > > +#else
> > > > +        unsigned char ttl = mcastTTL;
> > > > +#endif
> > > this "ifdef __linux__" feels like the wrong thing to check, dont you agree ?
> > 
> > Not sure what you mean.
> 
> "__linux__" seems the wrong property to check for
> this is not
> #ifdef __linux__
> osname = "linux"
> #else
> 
> i would have expected more something like
> #if HAVE_INT_TTL
> int ttl = mcastTTL;
> #else
> 
> or maybe even something along the lines of
> 
> WHATEVER_TTL_TYPE ttl = mcastTTL;
> 
> thx

Ok, how about something like this?


diff --git a/configure b/configure
index 493493b4c5..055ff3c206 100755
--- a/configure
+++ b/configure
@@ -3838,6 +3838,8 @@ doc_deps_any="manpages htmlpages podpages txtpages"
 
 logfile="ffbuild/config.log"
 
+multicast_ttl_type='unsigned char'
+
 # installation paths
 prefix_default="/usr/local"
 bindir_default='${prefix}/bin'
@@ -5653,6 +5655,7 @@ case $target_os in
     linux)
         enable section_data_rel_ro
         enabled_any arm aarch64 && enable_weak linux_perf
+        multicast_ttl_type='int'
         ;;
     irix*)
         target_os=irix
@@ -7784,6 +7787,7 @@ cat > $TMPH <<EOF
 #define SLIBSUF "$SLIBSUF"
 #define HAVE_MMX2 HAVE_MMXEXT
 #define SWS_MAX_FILTER_SIZE $sws_max_filter_size
+#define MULTICAST_TTL_TYPE $multicast_ttl_type
 EOF
 
 test -n "$assert_level" &&
diff --git a/libavformat/udp.c b/libavformat/udp.c
index 83c042d079..179e719286 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -27,6 +27,8 @@
 #define _DEFAULT_SOURCE
 #define _BSD_SOURCE     /* Needed for using struct ip_mreq with recent glibc */
 
+#include "config.h"
+
 #include "avformat.h"
 #include "avio_internal.h"
 #include "libavutil/avassert.h"
@@ -164,7 +166,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 {
 #ifdef IP_MULTICAST_TTL
     if (addr->sa_family == AF_INET) {
-        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
+        MULTICAST_TTL_TYPE ttl = mcastTTL;
+
+        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
             return ff_neterrno();
         }
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[lance.lmwang]

On Wed, Jan 12, 2022 at 12:13:13AM -0500, Brad Smith wrote:
> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
> type should be an unsigned char on anything but Linux.
> 
> 
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 180d96a988..29aa865fff 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>  {
>  #ifdef IP_MULTICAST_TTL
>      if (addr->sa_family == AF_INET) {
> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> +#ifdef __linux__
> +        int ttl = mcastTTL;
> +#else
> +        unsigned char ttl = mcastTTL;
> +#endif


I don't have BSD system for test, but I prefer to use socklen_t, please try with my proposal patch:

---
 libavformat/udp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 83c042d079..b9baa0a803 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 {
 #ifdef IP_MULTICAST_TTL
     if (addr->sa_family == AF_INET) {
-        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
+        socklen_t ttl = mcastTTL;
+
+        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
             return ff_neterrno();


> +
> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
>              ff_log_net_error(NULL, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>              return ff_neterrno();
>          }
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Chad Fraleigh]

Since apparently linux will auto-detect (as mentioned by Marton Balint), based on the optlen parameter, just using unsigned char in all cases seems to be the cleanest. However, I would advise including a comment in the code to that effect which says to ignore the [outdated] linux documentation (so someone doesn't needlessly "correct" it in the future).

I looked at the kernel source and it does work both ways:

static int do_ip_setsockopt(struct sock *sk, int level, int optname,
                sockptr_t optval, unsigned int optlen)
{
 ...
        switch (optname) {
 ...
        case IP_MULTICAST_TTL:
 ...
                if (optlen >= sizeof(int)) {
                        if (copy_from_sockptr(&val, optval, sizeof(val)))
                                return -EFAULT;
                } else if (optlen >= sizeof(char)) {
                        unsigned char ucval;

                        if (copy_from_sockptr(&ucval, optval, sizeof(ucval)))
                                return -EFAULT;
                        val = (int) ucval;
                }
        }
...
}

This check has been in the kernel since at least 2.6.12-rc2 (from Apr 2005). It should work fine, unless newer ffmpeg builds support is needed on older systems. So the only question is how old are the kernels in IoT and android devices which might use the current ffmpeg?


On 1/24/2022 11:25 PM, lance.lmwang@gmail.com wrote:
> On Wed, Jan 12, 2022 at 12:13:13AM -0500, Brad Smith wrote:
>> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
>> type should be an unsigned char on anything but Linux.
>>
>>
>> diff --git a/libavformat/udp.c b/libavformat/udp.c
>> index 180d96a988..29aa865fff 100644
>> --- a/libavformat/udp.c
>> +++ b/libavformat/udp.c
>> @@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>>  {
>>  #ifdef IP_MULTICAST_TTL
>>      if (addr->sa_family == AF_INET) {
>> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
>> +#ifdef __linux__
>> +        int ttl = mcastTTL;
>> +#else
>> +        unsigned char ttl = mcastTTL;
>> +#endif
> 
> 
> I don't have BSD system for test, but I prefer to use socklen_t, please try with my proposal patch:
> 
> ---
>  libavformat/udp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 83c042d079..b9baa0a803 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>  {
>  #ifdef IP_MULTICAST_TTL
>      if (addr->sa_family == AF_INET) {
> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> +        socklen_t ttl = mcastTTL;
> +
> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
>              ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>              return ff_neterrno();
> 
> 
>> +
>> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
>>              ff_log_net_error(NULL, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>>              return ff_neterrno();
>>          }
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Brad Smith]

On 1/25/2022 7:28 PM, Chad Fraleigh wrote:

> Since apparently linux will auto-detect (as mentioned by Marton Balint), based on the optlen parameter, just using unsigned char in all cases seems to be the cleanest. However, I would advise including a comment in the code to that effect which says to ignore the [outdated] linux documentation (so someone doesn't needlessly "correct" it in the future).
>
> I looked at the kernel source and it does work both ways:
>
> static int do_ip_setsockopt(struct sock *sk, int level, int optname,
>                  sockptr_t optval, unsigned int optlen)
> {
>   ...
>          switch (optname) {
>   ...
>          case IP_MULTICAST_TTL:
>   ...
>                  if (optlen >= sizeof(int)) {
>                          if (copy_from_sockptr(&val, optval, sizeof(val)))
>                                  return -EFAULT;
>                  } else if (optlen >= sizeof(char)) {
>                          unsigned char ucval;
>
>                          if (copy_from_sockptr(&ucval, optval, sizeof(ucval)))
>                                  return -EFAULT;
>                          val = (int) ucval;
>                  }
>          }
> ...
> }
>
> This check has been in the kernel since at least 2.6.12-rc2 (from Apr 2005). It should work fine, unless newer ffmpeg builds support is needed on older systems. So the only question is how old are the kernels in IoT and android devices which might use the current ffmpeg?

Thanks. This would he much simpler.

The oldest Android, 1.5, uses the 2.6.27 kernel. The oldest still 
supported Android, 4.4, uses the
3.10 kernel. I cannot imagine anyone shipping anything older than a 3.x 
(something like 3.18) kernel
on anything IoT that is still supported and wanting to ship up to date 
software.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[lance.lmwang]

On Tue, Jan 25, 2022 at 04:28:33PM -0800, Chad Fraleigh wrote:
> Since apparently linux will auto-detect (as mentioned by Marton Balint), based on the optlen parameter, just using unsigned char in all cases seems to be the cleanest. However, I would advise including a comment in the code to that effect which says to ignore the [outdated] linux documentation (so someone doesn't needlessly "correct" it in the future).
> 

I agree with, use unsigned char is preferable for all system I think.

> I looked at the kernel source and it does work both ways:
> 
> static int do_ip_setsockopt(struct sock *sk, int level, int optname,
>                 sockptr_t optval, unsigned int optlen)
> {
>  ...
>         switch (optname) {
>  ...
>         case IP_MULTICAST_TTL:
>  ...
>                 if (optlen >= sizeof(int)) {
>                         if (copy_from_sockptr(&val, optval, sizeof(val)))
>                                 return -EFAULT;
>                 } else if (optlen >= sizeof(char)) {
>                         unsigned char ucval;
> 
>                         if (copy_from_sockptr(&ucval, optval, sizeof(ucval)))
>                                 return -EFAULT;
>                         val = (int) ucval;
>                 }
>         }
> ...
> }
> 
> This check has been in the kernel since at least 2.6.12-rc2 (from Apr 2005). It should work fine, unless newer ffmpeg builds support is needed on older systems. So the only question is how old are the kernels in IoT and android devices which might use the current ffmpeg?
> 
> 
> On 1/24/2022 11:25 PM, lance.lmwang@gmail.com wrote:
> > On Wed, Jan 12, 2022 at 12:13:13AM -0500, Brad Smith wrote:
> >> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
> >> type should be an unsigned char on anything but Linux.
> >>
> >>
> >> diff --git a/libavformat/udp.c b/libavformat/udp.c
> >> index 180d96a988..29aa865fff 100644
> >> --- a/libavformat/udp.c
> >> +++ b/libavformat/udp.c
> >> @@ -163,7 +163,13 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >>  {
> >>  #ifdef IP_MULTICAST_TTL
> >>      if (addr->sa_family == AF_INET) {
> >> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> >> +#ifdef __linux__
> >> +        int ttl = mcastTTL;
> >> +#else
> >> +        unsigned char ttl = mcastTTL;
> >> +#endif
> > 
> > 
> > I don't have BSD system for test, but I prefer to use socklen_t, please try with my proposal patch:
> > 
> > ---
> >  libavformat/udp.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 83c042d079..b9baa0a803 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >  {
> >  #ifdef IP_MULTICAST_TTL
> >      if (addr->sa_family == AF_INET) {
> > -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > +        socklen_t ttl = mcastTTL;
> > +
> > +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
> >              ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
> >              return ff_neterrno();
> > 
> > 
> >> +
> >> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
> >>              ff_log_net_error(NULL, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
> >>              return ff_neterrno();
> >>          }
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Brad Smith]

On Wed, Jan 12, 2022 at 12:13:14AM -0500, Brad Smith wrote:
> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
> type should be an unsigned char on anything but Linux.

Based on feedback so far. Here is a much simpler approach to this issue..


diff --git a/libavformat/udp.c b/libavformat/udp.c
index 83c042d079..da1b98890b 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 {
 #ifdef IP_MULTICAST_TTL
     if (addr->sa_family == AF_INET) {
-        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
+        unsigned char ttl = mcastTTL;  /* ignore the outdated Linux documentation */
+
+        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
             return ff_neterrno();
         }
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Marton Balint]

On Wed, 26 Jan 2022, Brad Smith wrote:

> On Wed, Jan 12, 2022 at 12:13:14AM -0500, Brad Smith wrote:
>> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
>> type should be an unsigned char on anything but Linux.
>
> Based on feedback so far. Here is a much simpler approach to this issue..

Win32 needs DWORD unfortunately. I missed it earlier, sorry.

https://docs.microsoft.com/en-us/windows/win32/winsock/ipproto-ip-socket-options

Regards,
Marton

>
>
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 83c042d079..da1b98890b 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> {
> #ifdef IP_MULTICAST_TTL
>     if (addr->sa_family == AF_INET) {
> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> +        unsigned char ttl = mcastTTL;  /* ignore the outdated Linux documentation */
> +
> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
>             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>             return ff_neterrno();
>         }
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[Chad Fraleigh]

On 1/26/2022 12:50 PM, Marton Balint wrote:
> 
> 
> On Wed, 26 Jan 2022, Brad Smith wrote:
> 
>> On Wed, Jan 12, 2022 at 12:13:14AM -0500, Brad Smith wrote:
>>> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
>>> type should be an unsigned char on anything but Linux.
>>
>> Based on feedback so far. Here is a much simpler approach to this issue..
> 
> Win32 needs DWORD unfortunately. I missed it earlier, sorry.
> 
> https://docs.microsoft.com/en-us/windows/win32/winsock/ipproto-ip-socket-options

It gets worse from there. Since I can't look at the real Win32 source to see if it handles it both way, I had to go with Wine and ReactOS..

In Wine, the tests seem to use just an 'int' (which is probably always the same as 'DWORD'). But then, it proceeds to check it using a 1, 2, 3, and 4 byte optlen. In the implementation, it appears to just pass the values as-is to the underlying OS, rather than explicitly translate it to the appropriate type.

For ReactOS, it blindly treats it as an unsigned char (regardless of optlen, as long as it at least 1) when setting/getting the value. But then in a debug line just below that, it treats optval as an 'int *'.

Maybe some tests need to be done on Win32 to see if it works with either size (i.e. set as uchar, get as DWORD, then in reverse and make sure the input/output match in all cases).

I didn't check to see what msys/mingw* does, if anything.

> 
> Regards,
> Marton
> 
>>
>>
>> diff --git a/libavformat/udp.c b/libavformat/udp.c
>> index 83c042d079..da1b98890b 100644
>> --- a/libavformat/udp.c
>> +++ b/libavformat/udp.c
>> @@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>> {
>> #ifdef IP_MULTICAST_TTL
>>     if (addr->sa_family == AF_INET) {
>> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
>> +        unsigned char ttl = mcastTTL;  /* ignore the outdated Linux documentation */
>> +
>> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
>>             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>>             return ff_neterrno();
>>         }
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[lance.lmwang]

On Wed, Jan 26, 2022 at 09:50:47PM +0100, Marton Balint wrote:
> 
> 
> On Wed, 26 Jan 2022, Brad Smith wrote:
> 
> > On Wed, Jan 12, 2022 at 12:13:14AM -0500, Brad Smith wrote:
> > > Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
> > > type should be an unsigned char on anything but Linux.
> > 
> > Based on feedback so far. Here is a much simpler approach to this issue..
> 
> Win32 needs DWORD unfortunately. I missed it earlier, sorry.
> 
> https://docs.microsoft.com/en-us/windows/win32/winsock/ipproto-ip-socket-options

From my testing on Mac system, it support one byte only, but at least, it'll
report "Invalid argument" only if the ttl > 255. Maybe we can try with one byte
again if the errno is invalid like below. I tested it with ttl > 255 on Mac system
and without "Invalid argument" anymore.

 #ifdef IP_MULTICAST_TTL
+    int ret = 0;
     if (addr->sa_family == AF_INET) {
-        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
+        ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL));
+        /* try with byte also for IP_MULTICAST_TTL for some system like OpenBSD */
+        if (ret < 0 && errno == EINVAL) {
+            unsigned char ttl = mcastTTL;
+            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL): ");
+            ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl));
+        }
+        if (ret < 0) {
             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
             return ff_neterrno();


> 
> Regards,
> Marton
> 
> > 
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 83c042d079..da1b98890b 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> > {
> > #ifdef IP_MULTICAST_TTL
> >     if (addr->sa_family == AF_INET) {
> > -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > +        unsigned char ttl = mcastTTL;  /* ignore the outdated Linux documentation */
> > +
> > +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
> >             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
> >             return ff_neterrno();
> >         }
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

["zhilizhao(赵志立)"]

> On Jan 27, 2022, at 9:59 AM, lance.lmwang@gmail.com wrote:
> 
> On Wed, Jan 26, 2022 at 09:50:47PM +0100, Marton Balint wrote:
>> 
>> 
>> On Wed, 26 Jan 2022, Brad Smith wrote:
>> 
>>> On Wed, Jan 12, 2022 at 12:13:14AM -0500, Brad Smith wrote:
>>>> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
>>>> type should be an unsigned char on anything but Linux.
>>> 
>>> Based on feedback so far. Here is a much simpler approach to this issue..
>> 
>> Win32 needs DWORD unfortunately. I missed it earlier, sorry.
>> 
>> https://docs.microsoft.com/en-us/windows/win32/winsock/ipproto-ip-socket-options
> 
> From my testing on Mac system, it support one byte only, but at least, it'll
> report "Invalid argument" only if the ttl > 255. Maybe we can try with one byte
> again if the errno is invalid like below. I tested it with ttl > 255 on Mac system
> and without "Invalid argument" anymore.
> 
> #ifdef IP_MULTICAST_TTL
> +    int ret = 0;
>     if (addr->sa_family == AF_INET) {
> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> +        ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL));
> +        /* try with byte also for IP_MULTICAST_TTL for some system like OpenBSD */
> +        if (ret < 0 && errno == EINVAL) {
> +            unsigned char ttl = mcastTTL;
> +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL): ");
> +            ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl));
> +        }
> +        if (ret < 0) {
>             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>             return ff_neterrno();
> 
> 
>> 
>> Regards,
>> Marton
>> 
>>> 
>>> 
>>> diff --git a/libavformat/udp.c b/libavformat/udp.c
>>> index 83c042d079..da1b98890b 100644
>>> --- a/libavformat/udp.c
>>> +++ b/libavformat/udp.c
>>> @@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>>> {
>>> #ifdef IP_MULTICAST_TTL
>>>    if (addr->sa_family == AF_INET) {
>>> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
>>> +        unsigned char ttl = mcastTTL;  /* ignore the outdated Linux documentation */
>>> +
>>> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
>>>            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>>>            return ff_neterrno();
>>>        }
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> -- 
> Thanks,
> Limin Wang
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

["zhilizhao(赵志立)"]

> On Jan 27, 2022, at 9:59 AM, lance.lmwang@gmail.com wrote:
> 
> On Wed, Jan 26, 2022 at 09:50:47PM +0100, Marton Balint wrote:
>> 
>> 
>> On Wed, 26 Jan 2022, Brad Smith wrote:
>> 
>>> On Wed, Jan 12, 2022 at 12:13:14AM -0500, Brad Smith wrote:
>>>> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
>>>> type should be an unsigned char on anything but Linux.
>>> 
>>> Based on feedback so far. Here is a much simpler approach to this issue..
>> 
>> Win32 needs DWORD unfortunately. I missed it earlier, sorry.
>> 
>> https://docs.microsoft.com/en-us/windows/win32/winsock/ipproto-ip-socket-options
> 
> From my testing on Mac system, it support one byte only, but at least, it'll
> report "Invalid argument" only if the ttl > 255. Maybe we can try with one byte
> again if the errno is invalid like below. I tested it with ttl > 255 on Mac system
> and without "Invalid argument" anymore.


MacOS support int and unsigned char from my test. The upper bound of TTL is limited
to 255 (limited by protocol design), I guess it’s unrelated to int or unsigned char.


> 
> #ifdef IP_MULTICAST_TTL
> +    int ret = 0;
>     if (addr->sa_family == AF_INET) {
> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> +        ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL));
> +        /* try with byte also for IP_MULTICAST_TTL for some system like OpenBSD */
> +        if (ret < 0 && errno == EINVAL) {
> +            unsigned char ttl = mcastTTL;
> +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL): ");
> +            ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl));
> +        }
> +        if (ret < 0) {
>             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>             return ff_neterrno();
> 
> 
>> 
>> Regards,
>> Marton
>> 
>>> 
>>> 
>>> diff --git a/libavformat/udp.c b/libavformat/udp.c
>>> index 83c042d079..da1b98890b 100644
>>> --- a/libavformat/udp.c
>>> +++ b/libavformat/udp.c
>>> @@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>>> {
>>> #ifdef IP_MULTICAST_TTL
>>>    if (addr->sa_family == AF_INET) {
>>> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
>>> +        unsigned char ttl = mcastTTL;  /* ignore the outdated Linux documentation */
>>> +
>>> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
>>>            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
>>>            return ff_neterrno();
>>>        }
>>> _______________________________________________
>>> ffmpeg-devel mailing list
>>> ffmpeg-devel@ffmpeg.org
>>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>> 
>>> To unsubscribe, visit link above, or email
>>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>>> 
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>> 
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> -- 
> Thanks,
> Limin Wang
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH] Fix setsockopt IP_MULTICAST_TTL on OpenBSD

[lance.lmwang]

On Thu, Jan 27, 2022 at 10:30:10AM +0800, "zhilizhao(赵志立)" wrote:
> 
> 
> > On Jan 27, 2022, at 9:59 AM, lance.lmwang@gmail.com wrote:
> > 
> > On Wed, Jan 26, 2022 at 09:50:47PM +0100, Marton Balint wrote:
> >> 
> >> 
> >> On Wed, 26 Jan 2022, Brad Smith wrote:
> >> 
> >>> On Wed, Jan 12, 2022 at 12:13:14AM -0500, Brad Smith wrote:
> >>>> Fix setsockopt() usage on OpenBSD with IP_MULTICAST_TTL. The field
> >>>> type should be an unsigned char on anything but Linux.
> >>> 
> >>> Based on feedback so far. Here is a much simpler approach to this issue..
> >> 
> >> Win32 needs DWORD unfortunately. I missed it earlier, sorry.
> >> 
> >> https://docs.microsoft.com/en-us/windows/win32/winsock/ipproto-ip-socket-options
> > 
> > From my testing on Mac system, it support one byte only, but at least, it'll
> > report "Invalid argument" only if the ttl > 255. Maybe we can try with one byte
> > again if the errno is invalid like below. I tested it with ttl > 255 on Mac system
> > and without "Invalid argument" anymore.
> 
> 
> MacOS support int and unsigned char from my test. The upper bound of TTL is limited
> to 255 (limited by protocol design), I guess it’s unrelated to int or unsigned char.

Yes, MacOS isn't caused by one byte or int. By #Ticket9449, it'll failed for ttl=10,
so I guess my proposal which try with one byte again should be work for Openbsd system.
But you can't limit to 255 only as some system support for int like Win32.

> 
> 
> > 
> > #ifdef IP_MULTICAST_TTL
> > +    int ret = 0;
> >     if (addr->sa_family == AF_INET) {
> > -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > +        ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL));
> > +        /* try with byte also for IP_MULTICAST_TTL for some system like OpenBSD */
> > +        if (ret < 0 && errno == EINVAL) {
> > +            unsigned char ttl = mcastTTL;
> > +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL): ");
> > +            ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl));
> > +        }
> > +        if (ret < 0) {
> >             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
> >             return ff_neterrno();
> > 
> > 
> >> 
> >> Regards,
> >> Marton
> >> 
> >>> 
> >>> 
> >>> diff --git a/libavformat/udp.c b/libavformat/udp.c
> >>> index 83c042d079..da1b98890b 100644
> >>> --- a/libavformat/udp.c
> >>> +++ b/libavformat/udp.c
> >>> @@ -164,7 +164,9 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >>> {
> >>> #ifdef IP_MULTICAST_TTL
> >>>    if (addr->sa_family == AF_INET) {
> >>> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> >>> +        unsigned char ttl = mcastTTL;  /* ignore the outdated Linux documentation */
> >>> +
> >>> +        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl)) < 0) {
> >>>            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
> >>>            return ff_neterrno();
> >>>        }
> >>> _______________________________________________
> >>> ffmpeg-devel mailing list
> >>> ffmpeg-devel@ffmpeg.org
> >>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>> 
> >>> To unsubscribe, visit link above, or email
> >>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> >>> 
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >> 
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > 
> > -- 
> > Thanks,
> > Limin Wang
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Fix #ticket9449
Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
Make it as real patch so that you can test it easily as I don't have BSD system for testing.

 libavformat/udp.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 83c042d..a52a489 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -163,8 +163,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
                                  void *logctx)
 {
 #ifdef IP_MULTICAST_TTL
+    int ret = 0;
     if (addr->sa_family == AF_INET) {
-        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
+        ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL));
+        if (ret < 0 && errno == EINVAL) {
+            /* BSD compatibility */
+            unsigned char ttl = (unsigned char) ((mcastTTL > 255) ? 255 : mcastTTL);
+            ret = setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &ttl, sizeof(ttl));
+        }
+        if (ret < 0) {
             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
             return ff_neterrno();
         }
-- 
1.8.3.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH v2 1/3] avformat/udp: use one setsockopt for ipv4/ipv6

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/udp.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 83c042d..3dc79eb 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -162,22 +162,31 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
                                  struct sockaddr *addr,
                                  void *logctx)
 {
+    int protocol, cmd;
+
+    switch (addr->sa_family) {
 #ifdef IP_MULTICAST_TTL
-    if (addr->sa_family == AF_INET) {
-        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
-            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
-            return ff_neterrno();
-        }
-    }
+        case AF_INET:
+            protocol = IPPROTO_IP;
+            cmd      = IP_MULTICAST_TTL;
+            break;
 #endif
 #if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
-    if (addr->sa_family == AF_INET6) {
-        if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &mcastTTL, sizeof(mcastTTL)) < 0) {
-            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV6_MULTICAST_HOPS)");
+        case AF_INET6:
+            protocol = IPPROTO_IPV6;
+            cmd      = IPV6_MULTICAST_HOPS;
+            break;
+#endif
+        default:
+            errno = EAFNOSUPPORT;
             return ff_neterrno();
-        }
     }
-#endif
+
+    if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
+        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
+        return ff_neterrno();
+    }
+
     return 0;
 }
 
-- 
1.8.3.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH v2 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Suggested by zhilizhao, vlc project has solved the compatibility by
the same way, so I borrowed the comments from vlc project.

Fix #ticket9449

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/udp.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 3dc79eb..34488d6 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 {
     int protocol, cmd;
 
+    /* There is some confusion in the world whether IP_MULTICAST_TTL
+     * takes a byte or an int as an argument.
+     * BSD seems to indicate byte so we are going with that and use
+     * int as a fallback to be safe */
     switch (addr->sa_family) {
 #ifdef IP_MULTICAST_TTL
         case AF_INET:
@@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
     }
 
     if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
-        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
-        return ff_neterrno();
+        /* BSD compatibility */
+        unsigned char ttl;
+
+        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
+        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
+        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
+            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
+            return ff_neterrno();
+        }
     }
 
     return 0;
-- 
1.8.3.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH v2 3/3] avformat/udp: remove IPPROTO_IPV6 macro

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 34488d6..0b078d4 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -175,7 +175,7 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
             cmd      = IP_MULTICAST_TTL;
             break;
 #endif
-#if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
+#ifdef IPV6_MULTICAST_HOPS
         case AF_INET6:
             protocol = IPPROTO_IPV6;
             cmd      = IPV6_MULTICAST_HOPS;
-- 
1.8.3.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 1/3] avformat/udp: use one setsockopt for ipv4/ipv6

[Marton Balint]

On Sat, 5 Feb 2022, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavformat/udp.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 83c042d..3dc79eb 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -162,22 +162,31 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>                                  struct sockaddr *addr,
>                                  void *logctx)
> {
> +    int protocol, cmd;
> +
> +    switch (addr->sa_family) {
> #ifdef IP_MULTICAST_TTL
> -    if (addr->sa_family == AF_INET) {
> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> -            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
> -            return ff_neterrno();
> -        }
> -    }
> +        case AF_INET:
> +            protocol = IPPROTO_IP;
> +            cmd      = IP_MULTICAST_TTL;
> +            break;
> #endif
> #if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
> -    if (addr->sa_family == AF_INET6) {
> -        if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &mcastTTL, sizeof(mcastTTL)) < 0) {
> -            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV6_MULTICAST_HOPS)");
> +        case AF_INET6:
> +            protocol = IPPROTO_IPV6;
> +            cmd      = IPV6_MULTICAST_HOPS;
> +            break;
> +#endif
> +        default:
> +            errno = EAFNOSUPPORT;

This is not portable, ff_neterrno is different for winsock. Maybe you 
should simply remove the default case, to make it work like the old code, 
and not mix behaviour changes with factorization.

>             return ff_neterrno();
> -        }
>     }
> -#endif
> +
> +    if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> +        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");

"setsockopt(IPV4/IPV6 MULTICAST TTL)", so we know that the issue was with 
TTL setting?

Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[Marton Balint]

On Sat, 5 Feb 2022, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Suggested by zhilizhao, vlc project has solved the compatibility by
> the same way, so I borrowed the comments from vlc project.
>
> Fix #ticket9449
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavformat/udp.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 3dc79eb..34488d6 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> {
>     int protocol, cmd;
>
> +    /* There is some confusion in the world whether IP_MULTICAST_TTL
> +     * takes a byte or an int as an argument.
> +     * BSD seems to indicate byte so we are going with that and use
> +     * int as a fallback to be safe */

The code does the opposite. Tries int and falls back to byte. Either 
change code or change comment.

Regards,
Marton

>     switch (addr->sa_family) {
> #ifdef IP_MULTICAST_TTL
>         case AF_INET:
> @@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>     }
>
>     if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> -        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> -        return ff_neterrno();
> +        /* BSD compatibility */
> +        unsigned char ttl;
> +
> +        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
> +        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
> +        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> +            return ff_neterrno();
> +        }
>     }
>
>     return 0;
> -- 
> 1.8.3.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[lance.lmwang]

On Sat, Feb 05, 2022 at 10:59:55AM +0100, Marton Balint wrote:
> 
> 
> On Sat, 5 Feb 2022, lance.lmwang@gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Suggested by zhilizhao, vlc project has solved the compatibility by
> > the same way, so I borrowed the comments from vlc project.
> > 
> > Fix #ticket9449
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > libavformat/udp.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 3dc79eb..34488d6 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> > {
> >     int protocol, cmd;
> > 
> > +    /* There is some confusion in the world whether IP_MULTICAST_TTL
> > +     * takes a byte or an int as an argument.
> > +     * BSD seems to indicate byte so we are going with that and use
> > +     * int as a fallback to be safe */
> 
> The code does the opposite. Tries int and falls back to byte. Either change
> code or change comment.

Yes, good catch, so vlc use wrong comments. I'll change comments like below.
+     * BSD seems to indicate byte so we are going with that and tries
+     * int and falls back to byte to be safe */

> 
> Regards,
> Marton
> 
> >     switch (addr->sa_family) {
> > #ifdef IP_MULTICAST_TTL
> >         case AF_INET:
> > @@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >     }
> > 
> >     if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > -        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> > -        return ff_neterrno();
> > +        /* BSD compatibility */
> > +        unsigned char ttl;
> > +
> > +        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
> > +        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
> > +        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> > +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> > +            return ff_neterrno();
> > +        }
> >     }
> > 
> >     return 0;
> > -- 
> > 1.8.3.1
> > 
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 1/3] avformat/udp: use one setsockopt for ipv4/ipv6

[lance.lmwang]

On Sat, Feb 05, 2022 at 10:58:08AM +0100, Marton Balint wrote:
> 
> 
> On Sat, 5 Feb 2022, lance.lmwang@gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > libavformat/udp.c | 31 ++++++++++++++++++++-----------
> > 1 file changed, 20 insertions(+), 11 deletions(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 83c042d..3dc79eb 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -162,22 +162,31 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >                                  struct sockaddr *addr,
> >                                  void *logctx)
> > {
> > +    int protocol, cmd;
> > +
> > +    switch (addr->sa_family) {
> > #ifdef IP_MULTICAST_TTL
> > -    if (addr->sa_family == AF_INET) {
> > -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > -            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
> > -            return ff_neterrno();
> > -        }
> > -    }
> > +        case AF_INET:
> > +            protocol = IPPROTO_IP;
> > +            cmd      = IP_MULTICAST_TTL;
> > +            break;
> > #endif
> > #if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
> > -    if (addr->sa_family == AF_INET6) {
> > -        if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > -            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV6_MULTICAST_HOPS)");
> > +        case AF_INET6:
> > +            protocol = IPPROTO_IPV6;
> > +            cmd      = IPV6_MULTICAST_HOPS;
> > +            break;
> > +#endif
> > +        default:
> > +            errno = EAFNOSUPPORT;
> 
> This is not portable, ff_neterrno is different for winsock. Maybe you should
> simply remove the default case, to make it work like the old code, and not
> mix behaviour changes with factorization.

OK, will remove the default case to same with old code.

> 
> >             return ff_neterrno();
> > -        }
> >     }
> > -#endif
> > +
> > +    if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > +        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> 
> "setsockopt(IPV4/IPV6 MULTICAST TTL)", so we know that the issue was with
> TTL setting?

Sure, it looks better. will update the patch.

> 
> Thanks,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH v3 1/3] avformat/udp: use one setsockopt for ipv4/ipv6

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/udp.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 83c042d..8178d0e 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -162,22 +162,30 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
                                  struct sockaddr *addr,
                                  void *logctx)
 {
+    int protocol, cmd;
+
+    switch (addr->sa_family) {
 #ifdef IP_MULTICAST_TTL
-    if (addr->sa_family == AF_INET) {
-        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
-            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
-            return ff_neterrno();
-        }
-    }
+        case AF_INET:
+            protocol = IPPROTO_IP;
+            cmd      = IP_MULTICAST_TTL;
+            break;
 #endif
 #if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
-    if (addr->sa_family == AF_INET6) {
-        if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &mcastTTL, sizeof(mcastTTL)) < 0) {
-            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV6_MULTICAST_HOPS)");
-            return ff_neterrno();
-        }
-    }
+        case AF_INET6:
+            protocol = IPPROTO_IPV6;
+            cmd      = IPV6_MULTICAST_HOPS;
+            break;
 #endif
+        default:
+            return 0;
+    }
+
+    if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
+        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
+        return ff_neterrno();
+    }
+
     return 0;
 }
 
-- 
1.8.3.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH v3 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Suggested by zhilizhao, vlc project has solved the compatibility by
the same way, so I borrowed the comments from vlc project.

Fix #ticket9449

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/udp.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 8178d0e..1871acf 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 {
     int protocol, cmd;
 
+    /* There is some confusion in the world whether IP_MULTICAST_TTL
+     * takes a byte or an int as an argument.
+     * BSD seems to indicate byte so we are going with that and use
+     * int and fall back to byte to be safe */
     switch (addr->sa_family) {
 #ifdef IP_MULTICAST_TTL
         case AF_INET:
@@ -182,8 +186,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
     }
 
     if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
-        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
-        return ff_neterrno();
+        /* BSD compatibility */
+        unsigned char ttl;
+
+        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
+        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
+        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
+            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
+            return ff_neterrno();
+        }
     }
 
     return 0;
-- 
1.8.3.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

[FFmpeg-devel] [PATCH v3 3/3] avformat/udp: remove IPPROTO_IPV6 macro

[lance.lmwang]

From: Limin Wang <lance.lmwang@gmail.com>

Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/udp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 1871acf..9f6ab1b 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -175,7 +175,7 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
             cmd      = IP_MULTICAST_TTL;
             break;
 #endif
-#if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
+#ifdef IPV6_MULTICAST_HOPS
         case AF_INET6:
             protocol = IPPROTO_IPV6;
             cmd      = IPV6_MULTICAST_HOPS;
-- 
1.8.3.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[Chad Fraleigh]

Since any [valid] value over 255 is impossible in the IPv4 protocol (the TTL field is only 8-bits), it should always be capped at 255 (for consistency) or return an invalid value error (the one I would suggest).

Despite VLC's reversed comment, using an int seems to be the exception to the rule (i.e. only linux and windows seem to use it [as-documented]), perhaps doing the unsigned char first and using the int as the fallback would be better? It's not really just a BSD thing, unless you also count LWIP and Solaris as BSD. Unless VLC's code history shows them doing it this way at one time and swapping it (but forgetting the comment) to fix a known bug?


On 2/4/2022 9:28 PM, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Suggested by zhilizhao, vlc project has solved the compatibility by
> the same way, so I borrowed the comments from vlc project.
> 
> Fix #ticket9449
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavformat/udp.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 3dc79eb..34488d6 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>  {
>      int protocol, cmd;
>  
> +    /* There is some confusion in the world whether IP_MULTICAST_TTL
> +     * takes a byte or an int as an argument.
> +     * BSD seems to indicate byte so we are going with that and use
> +     * int as a fallback to be safe */
>      switch (addr->sa_family) {
>  #ifdef IP_MULTICAST_TTL
>          case AF_INET:
> @@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>      }
>  
>      if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> -        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> -        return ff_neterrno();
> +        /* BSD compatibility */
> +        unsigned char ttl;
> +
> +        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
> +        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
> +        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> +            return ff_neterrno();
> +        }
>      }
>  
>      return 0;
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[lance.lmwang]

On Sat, Feb 05, 2022 at 01:26:18PM -0800, Chad Fraleigh wrote:
> Since any [valid] value over 255 is impossible in the IPv4 protocol (the TTL field is only 8-bits), it should always be capped at 255 (for consistency) or return an invalid value error (the one I would suggest).
> 

zhilizhao have submit a patch to limit the range of ttl from option. Do you want
to return an invalid error here still?


> Despite VLC's reversed comment, using an int seems to be the exception to the rule (i.e. only linux and windows seem to use it [as-documented]), perhaps doing the unsigned char first and using the int as the fallback would be better? It's not really just a BSD thing, unless you also count LWIP and Solaris as BSD. Unless VLC's code history shows them doing it this way at one time and swapping it (but forgetting the comment) to fix a known bug?
> 

I have blamed vlc code and sure the code doing it this way at one time(104938796a3). 
For the mismatch of code and comments, I prefer to code always as code were build 
and used by all kinds of system which vlc is supported already. 

As for use BSD, I prefer to count LWIP and Solaris into BSD category which using
rule of byte. If you still prefer to add them into comments, I'm OK also. 

> 
> On 2/4/2022 9:28 PM, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Suggested by zhilizhao, vlc project has solved the compatibility by
> > the same way, so I borrowed the comments from vlc project.
> > 
> > Fix #ticket9449
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavformat/udp.c | 15 +++++++++++++--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 3dc79eb..34488d6 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >  {
> >      int protocol, cmd;
> >  
> > +    /* There is some confusion in the world whether IP_MULTICAST_TTL
> > +     * takes a byte or an int as an argument.
> > +     * BSD seems to indicate byte so we are going with that and use
> > +     * int as a fallback to be safe */
> >      switch (addr->sa_family) {
> >  #ifdef IP_MULTICAST_TTL
> >          case AF_INET:
> > @@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >      }
> >  
> >      if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > -        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> > -        return ff_neterrno();
> > +        /* BSD compatibility */
> > +        unsigned char ttl;
> > +
> > +        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
> > +        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
> > +        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> > +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> > +            return ff_neterrno();
> > +        }
> >      }
> >  
> >      return 0;
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[Marton Balint]

On Sun, 6 Feb 2022, lance.lmwang@gmail.com wrote:

> On Sat, Feb 05, 2022 at 01:26:18PM -0800, Chad Fraleigh wrote:
>> Since any [valid] value over 255 is impossible in the IPv4 protocol (the TTL field is only 8-bits), it should always be capped at 255 (for consistency) or return an invalid value error (the one I would suggest).
>>
>
> zhilizhao have submit a patch to limit the range of ttl from option. Do you want
> to return an invalid error here still?

I have applied the patch, so capping is no longer needed.

>
>
>> Despite VLC's reversed comment, using an int seems to be the exception 
>> to the rule (i.e. only linux and windows seem to use it 
>> [as-documented]), perhaps doing the unsigned char first and using the 
>> int as the fallback would be better? It's not really just a BSD thing, 
>> unless you also count LWIP and Solaris as BSD. Unless VLC's code 
>> history shows them doing it this way at one time and swapping it (but 
>> forgetting the comment) to fix a known bug?
>>
>
> I have blamed vlc code and sure the code doing it this way at one time(104938796a3).
> For the mismatch of code and comments, I prefer to code always as code were build
> and used by all kinds of system which vlc is supported already.
>

Yeah, I agree, if we are copying VLC approach then probably it makes more 
sense to use the same order as they do in their code.

Thanks,
Marton
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[Chad Fraleigh]

On 2/5/2022 6:09 PM, lance.lmwang@gmail.com wrote:
> On Sat, Feb 05, 2022 at 01:26:18PM -0800, Chad Fraleigh wrote:
>> Since any [valid] value over 255 is impossible in the IPv4 protocol (the TTL field is only 8-bits), it should always be capped at 255 (for consistency) or return an invalid value error (the one I would suggest).
>>
> 
> zhilizhao have submit a patch to limit the range of ttl from option. Do you want
> to return an invalid error here still?

If it can never be called with an invalid value, not even programmatically if someone links against the ffmpeg libs, then checking it is unneeded. But also checking it to limit the unsigned char value would be redundant, so only the value cast would be needed, i.e.:

   ttl = (unsigned char) mcastTTL;


If however, it could be called without being first limited, then returning an error would be best to avoid silently having unexpected results. Also, checking that it isn't negative should be done in that case. Not counting pending patches, I only see udp_open() calls it, so if it's already bound in there, no extra checks are needed.

Of course, these are only suggestions, since I'm a nobody. =)


>> Despite VLC's reversed comment, using an int seems to be the exception to the rule (i.e. only linux and windows seem to use it [as-documented]), perhaps doing the unsigned char first and using the int as the fallback would be better? It's not really just a BSD thing, unless you also count LWIP and Solaris as BSD. Unless VLC's code history shows them doing it this way at one time and swapping it (but forgetting the comment) to fix a known bug?
>>
> 
> I have blamed vlc code and sure the code doing it this way at one time(104938796a3). 
> For the mismatch of code and comments, I prefer to code always as code were build 
> and used by all kinds of system which vlc is supported already. 
> 
> As for use BSD, I prefer to count LWIP and Solaris into BSD category which using
> rule of byte. If you still prefer to add them into comments, I'm OK also. 
> 
>>
>> On 2/4/2022 9:28 PM, lance.lmwang@gmail.com wrote:
>>> From: Limin Wang <lance.lmwang@gmail.com>
>>>
>>> Suggested by zhilizhao, vlc project has solved the compatibility by
>>> the same way, so I borrowed the comments from vlc project.
>>>
>>> Fix #ticket9449
>>>
>>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
>>> ---
>>>  libavformat/udp.c | 15 +++++++++++++--
>>>  1 file changed, 13 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/libavformat/udp.c b/libavformat/udp.c
>>> index 3dc79eb..34488d6 100644
>>> --- a/libavformat/udp.c
>>> +++ b/libavformat/udp.c
>>> @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>>>  {
>>>      int protocol, cmd;
>>>  
>>> +    /* There is some confusion in the world whether IP_MULTICAST_TTL
>>> +     * takes a byte or an int as an argument.
>>> +     * BSD seems to indicate byte so we are going with that and use
>>> +     * int as a fallback to be safe */
>>>      switch (addr->sa_family) {
>>>  #ifdef IP_MULTICAST_TTL
>>>          case AF_INET:
>>> @@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>>>      }
>>>  
>>>      if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
>>> -        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
>>> -        return ff_neterrno();
>>> +        /* BSD compatibility */
>>> +        unsigned char ttl;
>>> +
>>> +        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
>>> +        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
>>> +        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
>>> +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
>>> +            return ff_neterrno();
>>> +        }
>>>      }
>>>  
>>>      return 0;
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> 
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v2 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[lance.lmwang]

On Sun, Feb 06, 2022 at 02:27:09PM -0800, Chad Fraleigh wrote:
> 
> On 2/5/2022 6:09 PM, lance.lmwang@gmail.com wrote:
> > On Sat, Feb 05, 2022 at 01:26:18PM -0800, Chad Fraleigh wrote:
> >> Since any [valid] value over 255 is impossible in the IPv4 protocol (the TTL field is only 8-bits), it should always be capped at 255 (for consistency) or return an invalid value error (the one I would suggest).
> >>
> > 
> > zhilizhao have submit a patch to limit the range of ttl from option. Do you want
> > to return an invalid error here still?
> 
> If it can never be called with an invalid value, not even programmatically if someone links against the ffmpeg libs, then checking it is unneeded. But also checking it to limit the unsigned char value would be redundant, so only the value cast would be needed, i.e.:
> 
>    ttl = (unsigned char) mcastTTL;

Yes, checking is unneeded anymore, I'll remove it locally.

> 
> 
> If however, it could be called without being first limited, then returning an error would be best to avoid silently having unexpected results. Also, checking that it isn't negative should be done in that case. Not counting pending patches, I only see udp_open() calls it, so if it's already bound in there, no extra checks are needed.

Sure, the patch will be applied after the pending patches for limit anyway if
nobody have other comments.

> 
> Of course, these are only suggestions, since I'm a nobody. =)
> 
> 
> >> Despite VLC's reversed comment, using an int seems to be the exception to the rule (i.e. only linux and windows seem to use it [as-documented]), perhaps doing the unsigned char first and using the int as the fallback would be better? It's not really just a BSD thing, unless you also count LWIP and Solaris as BSD. Unless VLC's code history shows them doing it this way at one time and swapping it (but forgetting the comment) to fix a known bug?
> >>
> > 
> > I have blamed vlc code and sure the code doing it this way at one time(104938796a3). 
> > For the mismatch of code and comments, I prefer to code always as code were build 
> > and used by all kinds of system which vlc is supported already. 
> > 
> > As for use BSD, I prefer to count LWIP and Solaris into BSD category which using
> > rule of byte. If you still prefer to add them into comments, I'm OK also. 
> > 
> >>
> >> On 2/4/2022 9:28 PM, lance.lmwang@gmail.com wrote:
> >>> From: Limin Wang <lance.lmwang@gmail.com>
> >>>
> >>> Suggested by zhilizhao, vlc project has solved the compatibility by
> >>> the same way, so I borrowed the comments from vlc project.
> >>>
> >>> Fix #ticket9449
> >>>
> >>> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> >>> ---
> >>>  libavformat/udp.c | 15 +++++++++++++--
> >>>  1 file changed, 13 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/libavformat/udp.c b/libavformat/udp.c
> >>> index 3dc79eb..34488d6 100644
> >>> --- a/libavformat/udp.c
> >>> +++ b/libavformat/udp.c
> >>> @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >>>  {
> >>>      int protocol, cmd;
> >>>  
> >>> +    /* There is some confusion in the world whether IP_MULTICAST_TTL
> >>> +     * takes a byte or an int as an argument.
> >>> +     * BSD seems to indicate byte so we are going with that and use
> >>> +     * int as a fallback to be safe */
> >>>      switch (addr->sa_family) {
> >>>  #ifdef IP_MULTICAST_TTL
> >>>          case AF_INET:
> >>> @@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >>>      }
> >>>  
> >>>      if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> >>> -        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> >>> -        return ff_neterrno();
> >>> +        /* BSD compatibility */
> >>> +        unsigned char ttl;
> >>> +
> >>> +        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
> >>> +        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
> >>> +        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> >>> +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> >>> +            return ff_neterrno();
> >>> +        }
> >>>      }
> >>>  
> >>>      return 0;
> >> _______________________________________________
> >> ffmpeg-devel mailing list
> >> ffmpeg-devel@ffmpeg.org
> >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>
> >> To unsubscribe, visit link above, or email
> >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v3 1/3] avformat/udp: use one setsockopt for ipv4/ipv6

[lance.lmwang]

On Sat, Feb 05, 2022 at 08:31:46PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavformat/udp.c | 32 ++++++++++++++++++++------------
>  1 file changed, 20 insertions(+), 12 deletions(-)
> 
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 83c042d..8178d0e 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -162,22 +162,30 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>                                   struct sockaddr *addr,
>                                   void *logctx)
>  {
> +    int protocol, cmd;
> +
> +    switch (addr->sa_family) {
>  #ifdef IP_MULTICAST_TTL
> -    if (addr->sa_family == AF_INET) {
> -        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, sizeof(mcastTTL)) < 0) {
> -            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_TTL)");
> -            return ff_neterrno();
> -        }
> -    }
> +        case AF_INET:
> +            protocol = IPPROTO_IP;
> +            cmd      = IP_MULTICAST_TTL;
> +            break;
>  #endif
>  #if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
> -    if (addr->sa_family == AF_INET6) {
> -        if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &mcastTTL, sizeof(mcastTTL)) < 0) {
> -            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV6_MULTICAST_HOPS)");
> -            return ff_neterrno();
> -        }
> -    }
> +        case AF_INET6:
> +            protocol = IPPROTO_IPV6;
> +            cmd      = IPV6_MULTICAST_HOPS;
> +            break;
>  #endif
> +        default:
> +            return 0;
> +    }
> +
> +    if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> +        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
> +        return ff_neterrno();
> +    }
> +
>      return 0;
>  }
>  
> -- 
> 1.8.3.1
> 

will apply tomorrow with one pending ttl range check patch if no further comments.

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[Marton Balint]

On Sat, 5 Feb 2022, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> Suggested by zhilizhao, vlc project has solved the compatibility by
> the same way, so I borrowed the comments from vlc project.
>
> Fix #ticket9449
>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavformat/udp.c | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 8178d0e..1871acf 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> {
>     int protocol, cmd;
>
> +    /* There is some confusion in the world whether IP_MULTICAST_TTL
> +     * takes a byte or an int as an argument.
> +     * BSD seems to indicate byte so we are going with that and use
> +     * int and fall back to byte to be safe */
>     switch (addr->sa_family) {
> #ifdef IP_MULTICAST_TTL
>         case AF_INET:
> @@ -182,8 +186,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
>     }
>
>     if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> -        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
> -        return ff_neterrno();
> +        /* BSD compatibility */
> +        unsigned char ttl;
> +
> +        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
> +        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);

I guess this limit check here is no longer needed after the range 
checking patches, so just remove. Otherwise LGTM.

Thanks,
Marton

> +        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
> +            return ff_neterrno();
> +        }
>     }
>
>     return 0;
> -- 
> 1.8.3.1
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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

Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

[lance.lmwang]

On Fri, Feb 11, 2022 at 10:05:19PM +0100, Marton Balint wrote:
> 
> 
> On Sat, 5 Feb 2022, lance.lmwang@gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > Suggested by zhilizhao, vlc project has solved the compatibility by
> > the same way, so I borrowed the comments from vlc project.
> > 
> > Fix #ticket9449
> > 
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> > libavformat/udp.c | 15 +++++++++++++--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 8178d0e..1871acf 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> > {
> >     int protocol, cmd;
> > 
> > +    /* There is some confusion in the world whether IP_MULTICAST_TTL
> > +     * takes a byte or an int as an argument.
> > +     * BSD seems to indicate byte so we are going with that and use
> > +     * int and fall back to byte to be safe */
> >     switch (addr->sa_family) {
> > #ifdef IP_MULTICAST_TTL
> >         case AF_INET:
> > @@ -182,8 +186,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
> >     }
> > 
> >     if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> > -        ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
> > -        return ff_neterrno();
> > +        /* BSD compatibility */
> > +        unsigned char ttl;
> > +
> > +        ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
> > +        ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
> 
> I guess this limit check here is no longer needed after the range checking
> patches, so just remove. Otherwise LGTM.

Yes, I have replied to Chad Fraleigh in another email and have removed the check already.

> 
> Thanks,
> Marton
> 
> > +        if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> > +            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST TTL)");
> > +            return ff_neterrno();
> > +        }
> >     }
> > 
> >     return 0;
> > -- 
> > 1.8.3.1
> > 
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > 
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
> > 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

-- 
Thanks,
Limin Wang
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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