[FFmpeg-devel] [avfomat/rtp: source ips lost when specified as URL options] Patch for ffmpeg using rtp protocol where sources option is not retained

[Ross Nicholson]

Thanks, just thought you might have another idea, as it works just was not sure if I was accomplishing it in the right way.

Ya, I submitted the patch formally. Let’s see what’s a review comes back like.

Thanks 

> On 26 Feb 2020, at 05:10, Jun Li <junli1026 at gmail.com> wrote:
> 
> 
> 
> 
>> On Tue, Feb 25, 2020 at 5:01 AM Ross Nicholson <phunkyfish at gmail.com> wrote:
>> Hey Jun Li,
>> 
>> I noticed you have submitted some patches which work around the same code area's that I submitted for. Your patches look quite tidy and well thought out so I was wondering if you could look at this patch and see if I'm going about it in the right way.
>> 
>> I'm not sure this area of ffmpeg currently has a maintainer currently so the patches may be difficult to progress.
>> 
>> Ross
>> 
>>> On Tue, 11 Feb 2020 at 22:42, Ross Nicholson <phunkyfish at gmail.com> wrote:
>>> The patch was created as a workaround to an issue from in kodi (apologies, it's a rather long thread): https://forum.kodi.tv/showthread.php?tid=350901&pid=2923550#pid2923550
>>> 
>>> As an example, here is a URL: rtp://87.141.215.251@232.0.10.234:10000
>>> 
>>> Taking this URL we should be able to either reformat it to: rtp://232.0.10.234:10000?sources=87.141.215.251 or pass the sources as an av_dict to avfomat_open_input.
>>> 
>>> Neither option works however. Instead the above workaround was created but it's not really the right way to fix this. Would be great to get some guidance on the right place to fix this in the right way.
>>> 
>>> Thanks in advance.
>>> 
>>>> On Tue, 11 Feb 2020 at 22:30, phunkyfish <phunkyfish at gmail.com> wrote:
>>>> ---
>>>>  libavformat/rtsp.c | 26 ++++++++++++++++++++++++--
>>>>  1 file changed, 24 insertions(+), 2 deletions(-)
>>>> 
>>>> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>>>> index 859defa592..f922055134 100644
>>>> --- a/libavformat/rtsp.c
>>>> +++ b/libavformat/rtsp.c
>>>> @@ -2334,7 +2334,9 @@ static int sdp_read_header(AVFormatContext *s)
>>>>      RTSPStream *rtsp_st;
>>>>      int size, i, err;
>>>>      char *content;
>>>> +    const char *p, *sp="", *sources="", *sp2, *sources2;
>>>>      char url[1024];
>>>> +    char sources_buf[1024];
>>>> 
>>>>      if (!ff_network_init())
>>>>          return AVERROR(EIO);
>>>> @@ -2360,6 +2362,16 @@ static int sdp_read_header(AVFormatContext *s)
>>>>      av_freep(&content);
>>>>      if (err) goto fail;
>>>> 
>>>> +    /* Search for sources= tag in original URL for rtp protocol only */
>>>> +    if (strncmp(s->url, "rtp://", 6) == 0) {
>>>> +        p = strchr(s->url, '?');
>>>> +        if (p && av_find_info_tag(sources_buf, sizeof(sources_buf), "sources", p)) {
>>>> +            /* av_log(s, AV_LOG_VERBOSE, "sdp_read_header found sources %s\n", sources_buf);  */
>>>> +            sp = sources_buf;
>>>> +            sources = "&sources=";
>>>> +        }
>>>> +    }
>>>> +
>>>>      /* open each RTP stream */
>>>>      for (i = 0; i < rt->nb_rtsp_streams; i++) {
>>>>          char namebuf[50];
>>>> @@ -2377,12 +2389,22 @@ static int sdp_read_header(AVFormatContext *s)
>>>>                  av_dict_free(&opts);
>>>>                  goto fail;
>>>>              }
>>>> +
>>>> +            /* Prepare to add sources to the url to be opened.
>>>> +               Otherwise the join to the source specific muliticast will be missing */
>>>> +            sources2 = sources;
>>>> +            sp2 = sp;
>>>> +            /* ignore sources from original URL, when sources are already set in rtsp_st */
>>>> +            if (rtsp_st->nb_include_source_addrs > 0)
>>>> +                sources2 = sp2 = "";
>>>> +
>>>>              ff_url_join(url, sizeof(url), "rtp", NULL,
>>>>                          namebuf, rtsp_st->sdp_port,
>>>> -                        "?localport=%d&ttl=%d&connect=%d&write_to_source=%d",
>>>> +                        "?localport=%d&ttl=%d&connect=%d&write_to_source=%d%s%s",
>>>>                          rtsp_st->sdp_port, rtsp_st->sdp_ttl,
>>>>                          rt->rtsp_flags & RTSP_FLAG_FILTER_SRC ? 1 : 0,
>>>> -                        rt->rtsp_flags & RTSP_FLAG_RTCP_TO_SOURCE ? 1 : 0);
>>>> +                        rt->rtsp_flags & RTSP_FLAG_RTCP_TO_SOURCE ? 1 : 0,
>>>> +                        sources2, sp2);
>>>> 
>>>>              append_source_addrs(url, sizeof(url), "sources",
>>>>                                  rtsp_st->nb_include_source_addrs,
>>>> -- 
>>>> 2.20.1 (Apple Git-117)
>>>> 
> 
> Hi Ross,
> I am not sure I understand your requirement clearly, an alternative way is to save the url in sdp(maybe in rtp_read_header?) and read it out in sdp_read_header. But I am not sure which field can carry this info (maybe uri attribute ?).
> 
> Since you already have the code change, why now send it as a formal patch so that FFmpeg maintainers/experts can have a review ?
> 
> -Jun