[FFmpeg-devel] [PATCH v5 3/3] avformat/whip: fix typos

[Timo Rothenpieler]

On 29.06.2025 15:18, Jack Lau wrote:
> 
> 
>> On Jun 29, 2025, at 19:47, Timo Rothenpieler <timo at rothenpieler.org> wrote:
>>
>> On 29.06.2025 05:14, Jack Lau wrote:
>>>> On Jun 29, 2025, at 11:11, Jack Lau <jacklau1222gm at gmail.com> wrote:
>>>>
>>>> Hi Timo,
>>>>> On Jun 29, 2025, at 06:49, Timo Rothenpieler <timo at rothenpieler.org> wrote:
>>>>>
>>>>> I've actually cleaned this up a bit while trying to implement DTLS via schannel, and effectively removed the whole section:
>>>>>
>>>>>> https://github.com/BtbN/FFmpeg/commit/b6794f1373fb07b791cfacd2189c7efc4d6bdbcc
>>>>>
>>>>> There's also a bunch of other necessary UDP related fixes in tls.c.
>>>>>
>>>>> Don't try to use an http proxy for UDP. It doesn't work:
>>>>>> https://github.com/BtbN/FFmpeg/commit/709ce9e5c48e3a27a400cf5af35038d3f0602c8a
>>>> I totally agree with the above two patches, it’s very reasonable
>>>>>
>>>>> Properly forward the various hosts and ports to udp.c so it actually works when using a non-external UDP connection:
>>>>>> https://github.com/BtbN/FFmpeg/commit/46375adf7d9cc61f709ab14dd2ea017995f735db
>>>> But I think this patch need a bit modify, I think the c->listen stands for FFmpeg if is server. When it is true, FFmpeg as server.
>>>> So maybe:
>>>>          if (c->listen) {
>>>>              av_dict_set_int(options, "localport", port, 0);
>>>>              av_dict_set(options, "localaddr", c->underlying_host, 0);Add commentMore actions
>>>>          } else {
>>>>              av_dict_set_int(options, "connect", 1, 0);
>>>>          }
>>> Sry for forgetting modify this code, the right code I want to express is that:
>>>          if (c->listen) {
>>>              av_dict_set_int(options, "connect", 1, 0);
>>>          } else {
>>>              av_dict_set_int(options, "localport", port, 0);
>>>              av_dict_set(options, "localaddr", c->underlying_host, 0);
>>>          }
>>
>> It goes together with
>>> https://github.com/BtbN/FFmpeg/commit/6fc902eb75554e6ad91a2ddf4ce1d131feee6f55
>>
>> Without setting the localport to 0 in client-mode in combination with above patch, it'll try to bind to the port passed in the URL, which when tryong to connect to a server on localhost will result in a bind failure, cause it tried to bind to the same port.
> I also found this issue, I’m not sure modify the udp code if is suitable, but I can provide a workaround:

The udp.c code clearly is lacking for this, so fixing it seems the right 
course of action to me.
We shouldn't be working around our own code.

> @@ -127,23 +127,22 @@ int ff_tls_open_underlying(TLSShared *c, URLContext *parent, const char *uri, AV
> 
> -    ret = ffurl_open_whitelist(c->is_dtls ? &c->udp : &c->tcp, buf, AVIO_FLAG_READ_WRITE,
> +    ret = ffurl_open_whitelist(c->is_dtls ? &c->udp : &c->tcp, buf,
> +                               c->is_dtls ? (c->listen ? AVIO_FLAG_READ : AVIO_FLAG_WRITE) : AVIO_FLAG_READ_WRITE,

That does seem like a crude hack-around to me at best.
Since a DTLS UDP socket will always be AVIO_FLAG_READ_WRITE, since the 
communications is unavoidably two-way.

It seems much better to me to adjust udp.c to properly support two-way 
communication.
There's also other issues, like the fifo it sets up for the packet 
processing thread will happily be used by both the udp_read and write 
functions if it's there, causing absolute chaos.
> https://github.com/FFmpeg/FFmpeg/commit/8d3096f7ac7c2a9193dac901a5f6b9d604a68882
In theory it could also launch both rx and tx queue now, but I focused 
on fixing the interference first.

There's other places where it clearly assumes to only ever be used for 
reading or writing, never both. DTLS might be the first instance of that 
no longer holding true.

You can see the full set of patches here:
> https://github.com/FFmpeg/FFmpeg/compare/master...BtbN:FFmpeg:schannel_dtls

There's a bunch more udp.c related enhancements and fixes.
Like, actually being able to find out who just sent a packet (schannel 
demands the clients sockaddr to do anything when in server mode).

The schannel dtls code works in client mode already (i.e. I can stream 
to an openssl s_server -dtls with it).
But for server mode _something_ is going wrong with loading the servers 
certificate, and I haven't figured out what it is yet.

>                                  &parent->interrupt_callback, options,
>                                  parent->protocol_whitelist, parent->protocol_blacklist, parent);
> diff --git a/libavformat/tls_openssl.c b/libavformat/tls_openssl.c
> index 2a3905891d..9cc5acd3e1 100644
> --- a/libavformat/tls_openssl.c
> +++ b/libavformat/tls_openssl.c
> @@ -985,6 +985,9 @@ static int dtls_start(URLContext *h, const char *url, int flags, AVDictionary **
>               av_log(p, AV_LOG_ERROR, "Failed to connect %s\n", url);
>               return ret;
>           }
> +        /* Make the socket non-blocking, set to READ and WRITE mode after connected */
> +        ff_socket_nonblock(ffurl_get_file_handle(p->tls_shared.udp), 1);
> +        p->tls_shared.udp->flags |= AVIO_FLAG_READ | AVIO_FLAG_WRITE | AVIO_FLAG_NONBLOCK;
>       }
>   
>>
>> And "connect" in listen mode makes no sense to me?
>> There is no remote address, where would it "connect" the socket to?
> Oops! I got it mixed up.
> You’re right.
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org <mailto:ffmpeg-devel at ffmpeg.org>
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org <mailto:ffmpeg-devel-request at ffmpeg.org> with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".