diff mbox series

[FFmpeg-devel,1/1] libavformat/rtsp: Pass protocol options for udp multicast

Message ID 20200403065350.31671-1-haupt.wolfgang@gmail.com
State Superseded
Headers show
Series [FFmpeg-devel,1/1] libavformat/rtsp: Pass protocol options for udp multicast | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Wolfgang Haupt April 3, 2020, 6:53 a.m. UTC 
Protocol options like buffer_size need to be passed to the
underlying transport implementation for udp multicasts as well.
---
 libavformat/rtsp.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Wolfgang Haupt April 19, 2020, 12:07 p.m. UTC | #1 
ping

On 03.04.20 08:53, Wolfgang Haupt wrote:
> Protocol options like buffer_size need to be passed to the
> underlying transport implementation for udp multicasts as well.
> ---
>   libavformat/rtsp.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index a69484d78b..dbf626eb13 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -1616,6 +1616,7 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
>               char url[1024], namebuf[50], optbuf[20] = "";
>               struct sockaddr_storage addr;
>               int port, ttl;
> +            AVDictionary *opts = map_to_opts(rt);
>   
>               if (reply->transports[0].destination.ss_family) {
>                   addr      = reply->transports[0].destination;
> @@ -1633,10 +1634,12 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
>               ff_url_join(url, sizeof(url), "rtp", NULL, namebuf,
>                           port, "%s", optbuf);
>               if (ffurl_open_whitelist(&rtsp_st->rtp_handle, url, AVIO_FLAG_READ_WRITE,
> -                           &s->interrupt_callback, NULL, s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
> +                           &s->interrupt_callback, &opts, s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
>                   err = AVERROR_INVALIDDATA;
> +                av_dict_free(&opts);
>                   goto fail;
>               }
> +            av_dict_free(&opts);
>               break;
>           }
>           }
Marton Balint April 19, 2020, 12:44 p.m. UTC | #2 
On Sun, 19 Apr 2020, Wolfgang Haupt wrote:

> ping
>
> On 03.04.20 08:53, Wolfgang Haupt wrote:
>> Protocol options like buffer_size need to be passed to the
>> underlying transport implementation for udp multicasts as well.
>> ---
>>   libavformat/rtsp.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>> index a69484d78b..dbf626eb13 100644
>> --- a/libavformat/rtsp.c
>> +++ b/libavformat/rtsp.c
>> @@ -1616,6 +1616,7 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, 
> const char *host, int port,
>>               char url[1024], namebuf[50], optbuf[20] = "";
>>               struct sockaddr_storage addr;
>>               int port, ttl;
>> +            AVDictionary *opts = map_to_opts(rt);
>>
>>               if (reply->transports[0].destination.ss_family) {
>>                   addr      = reply->transports[0].destination;
>> @@ -1633,10 +1634,12 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, 
> const char *host, int port,
>>               ff_url_join(url, sizeof(url), "rtp", NULL, namebuf,
>>                           port, "%s", optbuf);
>>               if (ffurl_open_whitelist(&rtsp_st->rtp_handle, url, 
> AVIO_FLAG_READ_WRITE,
>> -                           &s->interrupt_callback, NULL, 
> s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
>> +                           &s->interrupt_callback, &opts, 
> s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
>>                   err = AVERROR_INVALIDDATA;
>> +                av_dict_free(&opts);
>>                   goto fail;
>>               }
>> +            av_dict_free(&opts);

Can you rework this to something like

err = ff_url_open_whitelist()
av_dict_free(&opts)
if (err < 0) {
    err = AVERROR_INVALIDDATA;
    goto fail;
}

It frees opst in only one place.

Thanks,
Marton
Wolfgang Haupt April 19, 2020, 5:21 p.m. UTC | #3 
On 19.04.20 14:44, Marton Balint wrote:
>
>
> On Sun, 19 Apr 2020, Wolfgang Haupt wrote:
>
>> ping
>>
>> On 03.04.20 08:53, Wolfgang Haupt wrote:
>>> Protocol options like buffer_size need to be passed to the
>>> underlying transport implementation for udp multicasts as well.
>>> ---
>>>   libavformat/rtsp.c | 5 ++++-
>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>>> index a69484d78b..dbf626eb13 100644
>>> --- a/libavformat/rtsp.c
>>> +++ b/libavformat/rtsp.c
>>> @@ -1616,6 +1616,7 @@ int ff_rtsp_make_setup_request(AVFormatContext 
>>> *s, 
>> const char *host, int port,
>>>               char url[1024], namebuf[50], optbuf[20] = "";
>>>               struct sockaddr_storage addr;
>>>               int port, ttl;
>>> +            AVDictionary *opts = map_to_opts(rt);
>>>
>>>               if (reply->transports[0].destination.ss_family) {
>>>                   addr      = reply->transports[0].destination;
>>> @@ -1633,10 +1634,12 @@ int 
>>> ff_rtsp_make_setup_request(AVFormatContext *s, 
>> const char *host, int port,
>>>               ff_url_join(url, sizeof(url), "rtp", NULL, namebuf,
>>>                           port, "%s", optbuf);
>>>               if (ffurl_open_whitelist(&rtsp_st->rtp_handle, url, 
>> AVIO_FLAG_READ_WRITE,
>>> - &s->interrupt_callback, NULL, 
>> s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
>>> + &s->interrupt_callback, &opts, 
>> s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
>>>                   err = AVERROR_INVALIDDATA;
>>> +                av_dict_free(&opts);
>>>                   goto fail;
>>>               }
>>> +            av_dict_free(&opts);
>
> Can you rework this to something like
>
> err = ff_url_open_whitelist()
> av_dict_free(&opts)
> if (err < 0) {
>    err = AVERROR_INVALIDDATA;
>    goto fail;
> }
>
> It frees opst in only one place.

Done.
Sorry I clearly did something wrong with the submission of the new version.
It's shown 3 times now in patchwork.

>
> 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".
Marton Balint April 19, 2020, 9:50 p.m. UTC | #4 
On Sun, 19 Apr 2020, Wolfgang Haupt wrote:

> On 19.04.20 14:44, Marton Balint wrote:
>>
>>
>> On Sun, 19 Apr 2020, Wolfgang Haupt wrote:
>>
>>> ping
>>>
>>> On 03.04.20 08:53, Wolfgang Haupt wrote:
>>>> Protocol options like buffer_size need to be passed to the
>>>> underlying transport implementation for udp multicasts as well.
>>>> ---
>>>>   libavformat/rtsp.c | 5 ++++-
>>>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>>>> index a69484d78b..dbf626eb13 100644
>>>> --- a/libavformat/rtsp.c
>>>> +++ b/libavformat/rtsp.c
>>>> @@ -1616,6 +1616,7 @@ int ff_rtsp_make_setup_request(AVFormatContext 
>>>> *s, 
>>> const char *host, int port,
>>>>               char url[1024], namebuf[50], optbuf[20] = "";
>>>>               struct sockaddr_storage addr;
>>>>               int port, ttl;
>>>> +            AVDictionary *opts = map_to_opts(rt);
>>>>
>>>>               if (reply->transports[0].destination.ss_family) {
>>>>                   addr      = reply->transports[0].destination;
>>>> @@ -1633,10 +1634,12 @@ int 
>>>> ff_rtsp_make_setup_request(AVFormatContext *s, 
>>> const char *host, int port,
>>>>               ff_url_join(url, sizeof(url), "rtp", NULL, namebuf,
>>>>                           port, "%s", optbuf);
>>>>               if (ffurl_open_whitelist(&rtsp_st->rtp_handle, url, 
>>> AVIO_FLAG_READ_WRITE,
>>>> - &s->interrupt_callback, NULL, 
>>> s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
>>>> + &s->interrupt_callback, &opts, 
>>> s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
>>>>                   err = AVERROR_INVALIDDATA;
>>>> +                av_dict_free(&opts);
>>>>                   goto fail;
>>>>               }
>>>> +            av_dict_free(&opts);
>>
>> Can you rework this to something like
>>
>> err = ff_url_open_whitelist()
>> av_dict_free(&opts)
>> if (err < 0) {
>>    err = AVERROR_INVALIDDATA;
>>    goto fail;
>> }
>>
>> It frees opst in only one place.
>
> Done.
> Sorry I clearly did something wrong with the submission of the new version.
> It's shown 3 times now in patchwork.

No worries. To be frank I am not sure how to signal explictly that a patch 
supersedes anther one to patchwork.

Applied the patch with a minor cosmetic change.

Thanks,
Marton
diff mbox series

Patch

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index a69484d78b..dbf626eb13 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -1616,6 +1616,7 @@  int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
             char url[1024], namebuf[50], optbuf[20] = "";
             struct sockaddr_storage addr;
             int port, ttl;
+            AVDictionary *opts = map_to_opts(rt);
 
             if (reply->transports[0].destination.ss_family) {
                 addr      = reply->transports[0].destination;
@@ -1633,10 +1634,12 @@  int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
             ff_url_join(url, sizeof(url), "rtp", NULL, namebuf,
                         port, "%s", optbuf);
             if (ffurl_open_whitelist(&rtsp_st->rtp_handle, url, AVIO_FLAG_READ_WRITE,
-                           &s->interrupt_callback, NULL, s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
+                           &s->interrupt_callback, &opts, s->protocol_whitelist, s->protocol_blacklist, NULL) < 0) {
                 err = AVERROR_INVALIDDATA;
+                av_dict_free(&opts);
                 goto fail;
             }
+            av_dict_free(&opts);
             break;
         }
         }