On Sun, Feb 06, 2022 at 02:27:09PM -0800, Chad Fraleigh wrote:
>
> On 2/5/2022 6:09 PM, lance.lmw...@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.lmw...@gmail.com wrote:
> >>> From: Limin Wang <lance.lmw...@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.lmw...@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-requ...@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-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".
