Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
* [FFmpeg-devel] [PATCH v5 1/3] libavformat/rtsp: Make source specific multicast work for rtsp streams
@ 2025-02-26  4:01 Rashad Tatum
  2025-02-26  4:01 ` [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info Rashad Tatum
  2025-02-26  4:01 ` [FFmpeg-devel] [PATCH v5 3/3] libavformat/rtsp: Fix playback of sdp files from http urls Rashad Tatum
  0 siblings, 2 replies; 9+ messages in thread
From: Rashad Tatum @ 2025-02-26  4:01 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Rashad Tatum

by first changing the RTSPSource to track the destination address
obtained from the source filter. For each RTSPStream, only add the source
filter from the sdp if sdp_ip string matches source-filter's destination
address.

Before issuing the setup request, change the lower_transport to
multicast if the sdp_ip is a multicast address.

Change the multicast case to append sources (from the source-filter) to
the rtp url to make the source specific multicast join work.
---
 libavformat/rtsp.c | 88 +++++++++++++++++++++++++++++++---------------
 libavformat/rtsp.h |  3 +-
 2 files changed, 62 insertions(+), 29 deletions(-)

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 5ea471b40c..0c65f8d1a4 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -389,16 +389,18 @@ typedef struct SDPParseState {
 } SDPParseState;
 
 static void copy_default_source_addrs(struct RTSPSource **addrs, int count,
-                                      struct RTSPSource ***dest, int *dest_count)
+                                      struct RTSPSource ***dest, int *dest_count, const char* sdp_ip_str)
 {
     RTSPSource *rtsp_src, *rtsp_src2;
     int i;
     for (i = 0; i < count; i++) {
         rtsp_src = addrs[i];
-        rtsp_src2 = av_memdup(rtsp_src, sizeof(*rtsp_src));
-        if (!rtsp_src2)
-            continue;
-        dynarray_add(dest, dest_count, rtsp_src2);
+        if (strcmp(rtsp_src->dest_addr, sdp_ip_str) == 0) {
+            rtsp_src2 = av_memdup(rtsp_src, sizeof(*rtsp_src));
+            if (!rtsp_src2)
+                continue;
+            dynarray_add(dest, dest_count, rtsp_src2);
+        }
     }
 }
 
@@ -423,7 +425,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
 {
     RTSPState *rt = s->priv_data;
     char buf1[64], st_type[64];
-    const char *p;
+    const char *p, *sdp_ip_str, *dest_addr;
     enum AVMediaType codec_type;
     int payload_type;
     AVStream *st;
@@ -448,6 +450,8 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
         get_word_sep(buf1, sizeof(buf1), "/", &p);
         if (get_sockaddr(s, buf1, &sdp_ip))
             return;
+
+        sdp_ip_str = av_strdup(buf1);
         ttl = 16;
         if (*p == '/') {
             p++;
@@ -461,6 +465,18 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
             rtsp_st = rt->rtsp_streams[rt->nb_rtsp_streams - 1];
             rtsp_st->sdp_ip = sdp_ip;
             rtsp_st->sdp_ttl = ttl;
+
+            copy_default_source_addrs(s1->default_include_source_addrs,
+                                              s1->nb_default_include_source_addrs,
+                                              &rtsp_st->include_source_addrs,
+                                              &rtsp_st->nb_include_source_addrs,
+                                              sdp_ip_str);
+            copy_default_source_addrs(s1->default_exclude_source_addrs,
+                                              s1->nb_default_exclude_source_addrs,
+                                              &rtsp_st->exclude_source_addrs,
+                                              &rtsp_st->nb_exclude_source_addrs,
+                                              sdp_ip_str);
+
         }
         break;
     case 's':
@@ -504,15 +520,6 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
         rtsp_st->sdp_ip = s1->default_ip;
         rtsp_st->sdp_ttl = s1->default_ttl;
 
-        copy_default_source_addrs(s1->default_include_source_addrs,
-                                  s1->nb_default_include_source_addrs,
-                                  &rtsp_st->include_source_addrs,
-                                  &rtsp_st->nb_include_source_addrs);
-        copy_default_source_addrs(s1->default_exclude_source_addrs,
-                                  s1->nb_default_exclude_source_addrs,
-                                  &rtsp_st->exclude_source_addrs,
-                                  &rtsp_st->nb_exclude_source_addrs);
-
         get_word(buf1, sizeof(buf1), &p); /* port */
         rtsp_st->sdp_port = atoi(buf1);
 
@@ -672,11 +679,13 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
             // not checking that the destination address actually matches or is wildcard
             get_word(buf1, sizeof(buf1), &p);
 
+            dest_addr = av_strdup(buf1);
             while (*p != '\0') {
                 rtsp_src = av_mallocz(sizeof(*rtsp_src));
                 if (!rtsp_src)
                     return;
-                get_word(rtsp_src->addr, sizeof(rtsp_src->addr), &p);
+                av_strlcpy(rtsp_src->dest_addr, dest_addr, sizeof(rtsp_src->dest_addr));
+                get_word(rtsp_src->src_addr, sizeof(rtsp_src->src_addr), &p);
                 if (exclude) {
                     if (s->nb_streams == 0) {
                         dynarray_add(&s1->default_exclude_source_addrs, &s1->nb_default_exclude_source_addrs, rtsp_src);
@@ -859,10 +868,11 @@ int ff_rtsp_open_transport_ctx(AVFormatContext *s, RTSPStream *rtsp_st)
         rtsp_st->transport_priv = ff_rdt_parse_open(s, st->index,
                                             rtsp_st->dynamic_protocol_context,
                                             rtsp_st->dynamic_handler);
-    else if (CONFIG_RTPDEC)
+    else if (CONFIG_RTPDEC) {
         rtsp_st->transport_priv = ff_rtp_parse_open(s, st,
                                          rtsp_st->sdp_payload_type,
                                          reordering_queue_size);
+    }
 
     if (!rtsp_st->transport_priv) {
          return AVERROR(ENOMEM);
@@ -1452,6 +1462,18 @@ retry:
     return 0;
 }
 
+static void append_source_addrs(char *buf, int size, const char *name,
+                                int count, struct RTSPSource **addrs)
+{
+    int i;
+    if (!count)
+        return;
+    av_strlcatf(buf, size, "&%s=%s", name, addrs[0]->src_addr);
+    for (i = 1; i < count; i++) {
+        av_strlcatf(buf, size, ",%s", addrs[i]->src_addr);
+    }
+}
+
 int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
                               int lower_transport, const char *real_challenge)
 {
@@ -1510,6 +1532,11 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
         } else
             rtsp_st = rt->rtsp_streams[i];
 
+        /* RTP/UDP Source Specific Multicast (SSM) */
+        if (ff_is_multicast_address((struct sockaddr*) &rtsp_st->sdp_ip)) {
+            lower_transport = RTSP_LOWER_TRANSPORT_UDP_MULTICAST;
+        }
+
         /* RTP/UDP */
         if (lower_transport == RTSP_LOWER_TRANSPORT_UDP) {
             char buf[256];
@@ -1666,7 +1693,7 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
             break;
         }
         case RTSP_LOWER_TRANSPORT_UDP_MULTICAST: {
-            char url[MAX_URL_SIZE], namebuf[50], optbuf[20] = "";
+            char url[MAX_URL_SIZE], namebuf[50], optbuf[1024] = "";
             struct sockaddr_storage addr;
             int port, ttl;
             AVDictionary *opts = map_to_opts(rt);
@@ -1682,6 +1709,20 @@ int ff_rtsp_make_setup_request(AVFormatContext *s, const char *host, int port,
             }
             if (ttl > 0)
                 snprintf(optbuf, sizeof(optbuf), "?ttl=%d", ttl);
+
+            if (rtsp_st->nb_include_source_addrs > 0) {
+                if(strlen(optbuf) == 0 ) {
+                    snprintf(optbuf, sizeof(optbuf), "?");
+                }
+
+                append_source_addrs(optbuf, sizeof(optbuf), "sources",
+                                                rtsp_st->nb_include_source_addrs,
+                                                rtsp_st->include_source_addrs);
+                append_source_addrs(optbuf, sizeof(optbuf), "block",
+                                    rtsp_st->nb_exclude_source_addrs,
+                                    rtsp_st->exclude_source_addrs);
+            }
+
             getnameinfo((struct sockaddr*) &addr, sizeof(addr),
                         namebuf, sizeof(namebuf), NULL, 0, NI_NUMERICHOST);
             ff_url_join(url, sizeof(url), "rtp", NULL, namebuf,
@@ -2383,16 +2424,7 @@ static int sdp_probe(const AVProbeData *p1)
     return 0;
 }
 
-static void append_source_addrs(char *buf, int size, const char *name,
-                                int count, struct RTSPSource **addrs)
-{
-    int i;
-    if (!count)
-        return;
-    av_strlcatf(buf, size, "&%s=%s", name, addrs[0]->addr);
-    for (i = 1; i < count; i++)
-        av_strlcatf(buf, size, ",%s", addrs[i]->addr);
-}
+
 
 static int sdp_read_header(AVFormatContext *s)
 {
diff --git a/libavformat/rtsp.h b/libavformat/rtsp.h
index 83b2e3f4fb..61de5cfb17 100644
--- a/libavformat/rtsp.h
+++ b/libavformat/rtsp.h
@@ -432,7 +432,8 @@ typedef struct RTSPState {
 #define RTSP_FLAG_SATIP_RAW   0x20   /**< Export SAT>IP stream as raw MPEG-TS */
 
 typedef struct RTSPSource {
-    char addr[128]; /**< Source-specific multicast include source IP address (from SDP content) */
+    char dest_addr[128]; /**< Source-specific multicast destination IP address (from SDP content) */
+    char src_addr[128]; /**< Source-specific multicast include source IP address (from SDP content) */
 } RTSPSource;
 
 /**
-- 
2.48.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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info
  2025-02-26  4:01 [FFmpeg-devel] [PATCH v5 1/3] libavformat/rtsp: Make source specific multicast work for rtsp streams Rashad Tatum
@ 2025-02-26  4:01 ` Rashad Tatum
  2025-02-26  8:52   ` Andreas Rheinhardt
  2025-02-26  4:01 ` [FFmpeg-devel] [PATCH v5 3/3] libavformat/rtsp: Fix playback of sdp files from http urls Rashad Tatum
  1 sibling, 1 reply; 9+ messages in thread
From: Rashad Tatum @ 2025-02-26  4:01 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Rashad Tatum

---
 libavformat/rtsp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index 0c65f8d1a4..ac17717195 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -478,6 +478,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
                                               sdp_ip_str);
 
         }
+        av_freep(&sdp_ip_str);
         break;
     case 's':
         av_dict_set(&s->metadata, "title", p, 0);
@@ -702,6 +703,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
                     }
                 }
             }
+            av_freep(&dest_addr);
         } else {
             if (rt->server_type == RTSP_SERVER_WMS)
                 ff_wms_parse_sdp_a_line(s, p);
-- 
2.48.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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [FFmpeg-devel] [PATCH v5 3/3] libavformat/rtsp: Fix playback of sdp files from http urls.
  2025-02-26  4:01 [FFmpeg-devel] [PATCH v5 1/3] libavformat/rtsp: Make source specific multicast work for rtsp streams Rashad Tatum
  2025-02-26  4:01 ` [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info Rashad Tatum
@ 2025-02-26  4:01 ` Rashad Tatum
  1 sibling, 0 replies; 9+ messages in thread
From: Rashad Tatum @ 2025-02-26  4:01 UTC (permalink / raw)
  To: ffmpeg-devel; +Cc: Rashad Tatum

Support wildcard source-filter destinations.
Revamp source-filter processing to only happen after the first
media description was processed and either when the next one starts
or when we have reached the end of the sdp.

This handles a wider variety of cases and doesn't rely on any optional
media descriptions.
---
 libavformat/rtsp.c | 46 ++++++++++++++++++++++++++++++----------------
 1 file changed, 30 insertions(+), 16 deletions(-)

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index ac17717195..1fbcb20167 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -19,6 +19,8 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include <stdbool.h>
+
 #include "config_components.h"
 
 #include "libavutil/avassert.h"
@@ -393,9 +395,10 @@ static void copy_default_source_addrs(struct RTSPSource **addrs, int count,
 {
     RTSPSource *rtsp_src, *rtsp_src2;
     int i;
+    const char* wildcard_dest = "*";
     for (i = 0; i < count; i++) {
         rtsp_src = addrs[i];
-        if (strcmp(rtsp_src->dest_addr, sdp_ip_str) == 0) {
+        if ((strcmp(rtsp_src->dest_addr, sdp_ip_str) == 0) || (strcmp(rtsp_src->dest_addr, wildcard_dest) == 0)) {
             rtsp_src2 = av_memdup(rtsp_src, sizeof(*rtsp_src));
             if (!rtsp_src2)
                 continue;
@@ -425,7 +428,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
 {
     RTSPState *rt = s->priv_data;
     char buf1[64], st_type[64];
-    const char *p, *sdp_ip_str, *dest_addr;
+    const char *p, *dest_addr;
     enum AVMediaType codec_type;
     int payload_type;
     AVStream *st;
@@ -451,7 +454,6 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
         if (get_sockaddr(s, buf1, &sdp_ip))
             return;
 
-        sdp_ip_str = av_strdup(buf1);
         ttl = 16;
         if (*p == '/') {
             p++;
@@ -466,19 +468,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
             rtsp_st->sdp_ip = sdp_ip;
             rtsp_st->sdp_ttl = ttl;
 
-            copy_default_source_addrs(s1->default_include_source_addrs,
-                                              s1->nb_default_include_source_addrs,
-                                              &rtsp_st->include_source_addrs,
-                                              &rtsp_st->nb_include_source_addrs,
-                                              sdp_ip_str);
-            copy_default_source_addrs(s1->default_exclude_source_addrs,
-                                              s1->nb_default_exclude_source_addrs,
-                                              &rtsp_st->exclude_source_addrs,
-                                              &rtsp_st->nb_exclude_source_addrs,
-                                              sdp_ip_str);
-
         }
-        av_freep(&sdp_ip_str);
         break;
     case 's':
         av_dict_set(&s->metadata, "title", p, 0);
@@ -521,6 +511,8 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
         rtsp_st->sdp_ip = s1->default_ip;
         rtsp_st->sdp_ttl = s1->default_ttl;
 
+
+
         get_word(buf1, sizeof(buf1), &p); /* port */
         rtsp_st->sdp_port = atoi(buf1);
 
@@ -728,13 +720,31 @@ int ff_sdp_parse(AVFormatContext *s, const char *content)
 {
     const char *p;
     int letter, i;
-    char buf[SDP_MAX_SIZE], *q;
+    char buf[SDP_MAX_SIZE], sdp_ip_str[50], *q;
+    bool parsed_first_media_desc = false;
     SDPParseState sdp_parse_state = { { 0 } }, *s1 = &sdp_parse_state;
 
     p = content;
     for (;;) {
         p += strspn(p, SPACE_CHARS);
         letter = *p;
+        if(parsed_first_media_desc && (letter == 'm' || letter == '\0')) {
+            RTSPState *rt = s->priv_data;
+            RTSPStream *rtsp_st = rt->rtsp_streams[rt->nb_rtsp_streams - 1];
+            getnameinfo((struct sockaddr*) &rtsp_st->sdp_ip,
+                                            sizeof(rtsp_st->sdp_ip),
+                                            sdp_ip_str, sizeof(sdp_ip_str), NULL, 0, NI_NUMERICHOST);
+            copy_default_source_addrs(s1->default_include_source_addrs,
+                                            s1->nb_default_include_source_addrs,
+                                            &rtsp_st->include_source_addrs,
+                                            &rtsp_st->nb_include_source_addrs,
+                                            sdp_ip_str);
+            copy_default_source_addrs(s1->default_exclude_source_addrs,
+                                            s1->nb_default_exclude_source_addrs,
+                                            &rtsp_st->exclude_source_addrs,
+                                            &rtsp_st->nb_exclude_source_addrs,
+                                            sdp_ip_str);
+        }
         if (letter == '\0')
             break;
         p++;
@@ -749,7 +759,11 @@ int ff_sdp_parse(AVFormatContext *s, const char *content)
             p++;
         }
         *q = '\0';
+
         sdp_parse_line(s, s1, letter, buf);
+        if(letter == 'm')
+            parsed_first_media_desc = true;
+
     next_line:
         while (*p != '\n' && *p != '\0')
             p++;
-- 
2.48.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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info
  2025-02-26  4:01 ` [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info Rashad Tatum
@ 2025-02-26  8:52   ` Andreas Rheinhardt
  2025-02-26 12:14     ` Rashad Tatum
  0 siblings, 1 reply; 9+ messages in thread
From: Andreas Rheinhardt @ 2025-02-26  8:52 UTC (permalink / raw)
  To: ffmpeg-devel

Rashad Tatum:
> ---
>  libavformat/rtsp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index 0c65f8d1a4..ac17717195 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -478,6 +478,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
>                                                sdp_ip_str);
>  
>          }
> +        av_freep(&sdp_ip_str);
>          break;
>      case 's':
>          av_dict_set(&s->metadata, "title", p, 0);
> @@ -702,6 +703,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
>                      }
>                  }
>              }
> +            av_freep(&dest_addr);
>          } else {
>              if (rt->server_type == RTSP_SERVER_WMS)
>                  ff_wms_parse_sdp_a_line(s, p);

Your first patch introduces a memleak; the stuff you allocated there
should be freed in the patch, not fixed up in a later patch.
(Also note that there must not be a leak in any error path, which is not
true when your patchset is applied: dest_addr leaks when rtsp_src can't
be allocated.)

- Andreas

_______________________________________________
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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info
  2025-02-26  8:52   ` Andreas Rheinhardt
@ 2025-02-26 12:14     ` Rashad Tatum
  2025-02-26 12:52       ` Rashad Tatum
  0 siblings, 1 reply; 9+ messages in thread
From: Rashad Tatum @ 2025-02-26 12:14 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

Ok. I'll move this and the third patch to one commit/patch and also change
the condition when rtsp_src can't be allocated to also free dest_addr. Good
catch.


On Wed, Feb 26, 2025, 3:53 AM Andreas Rheinhardt <
andreas.rheinhardt@outlook.com> wrote:

> Rashad Tatum:
> > ---
> >  libavformat/rtsp.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index 0c65f8d1a4..ac17717195 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -478,6 +478,7 @@ static void sdp_parse_line(AVFormatContext *s,
> SDPParseState *s1,
> >                                                sdp_ip_str);
> >
> >          }
> > +        av_freep(&sdp_ip_str);
> >          break;
> >      case 's':
> >          av_dict_set(&s->metadata, "title", p, 0);
> > @@ -702,6 +703,7 @@ static void sdp_parse_line(AVFormatContext *s,
> SDPParseState *s1,
> >                      }
> >                  }
> >              }
> > +            av_freep(&dest_addr);
> >          } else {
> >              if (rt->server_type == RTSP_SERVER_WMS)
> >                  ff_wms_parse_sdp_a_line(s, p);
>
> Your first patch introduces a memleak; the stuff you allocated there
> should be freed in the patch, not fixed up in a later patch.
> (Also note that there must not be a leak in any error path, which is not
> true when your patchset is applied: dest_addr leaks when rtsp_src can't
> be allocated.)
>
> - Andreas
>
> _______________________________________________
> 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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info
  2025-02-26 12:14     ` Rashad Tatum
@ 2025-02-26 12:52       ` Rashad Tatum
  2025-02-26 13:16         ` Rashad Tatum
  0 siblings, 1 reply; 9+ messages in thread
From: Rashad Tatum @ 2025-02-26 12:52 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

As a side note,  I think the failure of the rtsp_src memory allocation
should also log an error so that the user is aware of why the rtsp stream
failed to play. However, I think this change should be part of separate
patch series.

On Wed, Feb 26, 2025, 7:14 AM Rashad Tatum <tatum.rashad@gmail.com> wrote:

> Ok. I'll move this and the third patch to one commit/patch and also change
> the condition when rtsp_src can't be allocated to also free dest_addr. Good
> catch.
>
>
> On Wed, Feb 26, 2025, 3:53 AM Andreas Rheinhardt <
> andreas.rheinhardt@outlook.com> wrote:
>
>> Rashad Tatum:
>> > ---
>> >  libavformat/rtsp.c | 2 ++
>> >  1 file changed, 2 insertions(+)
>> >
>> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>> > index 0c65f8d1a4..ac17717195 100644
>> > --- a/libavformat/rtsp.c
>> > +++ b/libavformat/rtsp.c
>> > @@ -478,6 +478,7 @@ static void sdp_parse_line(AVFormatContext *s,
>> SDPParseState *s1,
>> >                                                sdp_ip_str);
>> >
>> >          }
>> > +        av_freep(&sdp_ip_str);
>> >          break;
>> >      case 's':
>> >          av_dict_set(&s->metadata, "title", p, 0);
>> > @@ -702,6 +703,7 @@ static void sdp_parse_line(AVFormatContext *s,
>> SDPParseState *s1,
>> >                      }
>> >                  }
>> >              }
>> > +            av_freep(&dest_addr);
>> >          } else {
>> >              if (rt->server_type == RTSP_SERVER_WMS)
>> >                  ff_wms_parse_sdp_a_line(s, p);
>>
>> Your first patch introduces a memleak; the stuff you allocated there
>> should be freed in the patch, not fixed up in a later patch.
>> (Also note that there must not be a leak in any error path, which is not
>> true when your patchset is applied: dest_addr leaks when rtsp_src can't
>> be allocated.)
>>
>> - Andreas
>>
>> _______________________________________________
>> 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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info
  2025-02-26 12:52       ` Rashad Tatum
@ 2025-02-26 13:16         ` Rashad Tatum
  2025-02-26 13:18           ` Rashad Tatum
  0 siblings, 1 reply; 9+ messages in thread
From: Rashad Tatum @ 2025-02-26 13:16 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

In other parts of the ffmpeg code base where the return type is not void
for the function, NULL is returned and then the caller typically returns
AVERROR.

Strangely, sdp_parse_line is void and it seems like ff_sdp_parse always
returns 0, despite the caller (sdp_read_header) attempting to handle any
potential returned error codes.

On Wed, Feb 26, 2025, 7:52 AM Rashad Tatum <tatum.rashad@gmail.com> wrote:

> As a side note,  I think the failure of the rtsp_src memory allocation
> should also log an error so that the user is aware of why the rtsp stream
> failed to play. However, I think this change should be part of separate
> patch series.
>
> On Wed, Feb 26, 2025, 7:14 AM Rashad Tatum <tatum.rashad@gmail.com> wrote:
>
>> Ok. I'll move this and the third patch to one commit/patch and also
>> change the condition when rtsp_src can't be allocated to also free
>> dest_addr. Good catch.
>>
>>
>> On Wed, Feb 26, 2025, 3:53 AM Andreas Rheinhardt <
>> andreas.rheinhardt@outlook.com> wrote:
>>
>>> Rashad Tatum:
>>> > ---
>>> >  libavformat/rtsp.c | 2 ++
>>> >  1 file changed, 2 insertions(+)
>>> >
>>> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>>> > index 0c65f8d1a4..ac17717195 100644
>>> > --- a/libavformat/rtsp.c
>>> > +++ b/libavformat/rtsp.c
>>> > @@ -478,6 +478,7 @@ static void sdp_parse_line(AVFormatContext *s,
>>> SDPParseState *s1,
>>> >                                                sdp_ip_str);
>>> >
>>> >          }
>>> > +        av_freep(&sdp_ip_str);
>>> >          break;
>>> >      case 's':
>>> >          av_dict_set(&s->metadata, "title", p, 0);
>>> > @@ -702,6 +703,7 @@ static void sdp_parse_line(AVFormatContext *s,
>>> SDPParseState *s1,
>>> >                      }
>>> >                  }
>>> >              }
>>> > +            av_freep(&dest_addr);
>>> >          } else {
>>> >              if (rt->server_type == RTSP_SERVER_WMS)
>>> >                  ff_wms_parse_sdp_a_line(s, p);
>>>
>>> Your first patch introduces a memleak; the stuff you allocated there
>>> should be freed in the patch, not fixed up in a later patch.
>>> (Also note that there must not be a leak in any error path, which is not
>>> true when your patchset is applied: dest_addr leaks when rtsp_src can't
>>> be allocated.)
>>>
>>> - Andreas
>>>
>>> _______________________________________________
>>> 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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info
  2025-02-26 13:16         ` Rashad Tatum
@ 2025-02-26 13:18           ` Rashad Tatum
  2025-02-26 13:19             ` Rashad Tatum
  0 siblings, 1 reply; 9+ messages in thread
From: Rashad Tatum @ 2025-02-26 13:18 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

So, perhaps sdp_parse_line should not be void and should propagate an error
code up that should be returned by ff_sdp_parse.

On Wed, Feb 26, 2025, 8:16 AM Rashad Tatum <tatum.rashad@gmail.com> wrote:

> In other parts of the ffmpeg code base where the return type is not void
> for the function, NULL is returned and then the caller typically returns
> AVERROR.
>
> Strangely, sdp_parse_line is void and it seems like ff_sdp_parse always
> returns 0, despite the caller (sdp_read_header) attempting to handle any
> potential returned error codes.
>
> On Wed, Feb 26, 2025, 7:52 AM Rashad Tatum <tatum.rashad@gmail.com> wrote:
>
>> As a side note,  I think the failure of the rtsp_src memory allocation
>> should also log an error so that the user is aware of why the rtsp stream
>> failed to play. However, I think this change should be part of separate
>> patch series.
>>
>> On Wed, Feb 26, 2025, 7:14 AM Rashad Tatum <tatum.rashad@gmail.com>
>> wrote:
>>
>>> Ok. I'll move this and the third patch to one commit/patch and also
>>> change the condition when rtsp_src can't be allocated to also free
>>> dest_addr. Good catch.
>>>
>>>
>>> On Wed, Feb 26, 2025, 3:53 AM Andreas Rheinhardt <
>>> andreas.rheinhardt@outlook.com> wrote:
>>>
>>>> Rashad Tatum:
>>>> > ---
>>>> >  libavformat/rtsp.c | 2 ++
>>>> >  1 file changed, 2 insertions(+)
>>>> >
>>>> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>>>> > index 0c65f8d1a4..ac17717195 100644
>>>> > --- a/libavformat/rtsp.c
>>>> > +++ b/libavformat/rtsp.c
>>>> > @@ -478,6 +478,7 @@ static void sdp_parse_line(AVFormatContext *s,
>>>> SDPParseState *s1,
>>>> >                                                sdp_ip_str);
>>>> >
>>>> >          }
>>>> > +        av_freep(&sdp_ip_str);
>>>> >          break;
>>>> >      case 's':
>>>> >          av_dict_set(&s->metadata, "title", p, 0);
>>>> > @@ -702,6 +703,7 @@ static void sdp_parse_line(AVFormatContext *s,
>>>> SDPParseState *s1,
>>>> >                      }
>>>> >                  }
>>>> >              }
>>>> > +            av_freep(&dest_addr);
>>>> >          } else {
>>>> >              if (rt->server_type == RTSP_SERVER_WMS)
>>>> >                  ff_wms_parse_sdp_a_line(s, p);
>>>>
>>>> Your first patch introduces a memleak; the stuff you allocated there
>>>> should be freed in the patch, not fixed up in a later patch.
>>>> (Also note that there must not be a leak in any error path, which is not
>>>> true when your patchset is applied: dest_addr leaks when rtsp_src can't
>>>> be allocated.)
>>>>
>>>> - Andreas
>>>>
>>>> _______________________________________________
>>>> 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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info
  2025-02-26 13:18           ` Rashad Tatum
@ 2025-02-26 13:19             ` Rashad Tatum
  0 siblings, 0 replies; 9+ messages in thread
From: Rashad Tatum @ 2025-02-26 13:19 UTC (permalink / raw)
  To: FFmpeg development discussions and patches

I'll submit a separate patch for this issue

On Wed, Feb 26, 2025, 8:18 AM Rashad Tatum <tatum.rashad@gmail.com> wrote:

> So, perhaps sdp_parse_line should not be void and should propagate an
> error code up that should be returned by ff_sdp_parse.
>
> On Wed, Feb 26, 2025, 8:16 AM Rashad Tatum <tatum.rashad@gmail.com> wrote:
>
>> In other parts of the ffmpeg code base where the return type is not void
>> for the function, NULL is returned and then the caller typically returns
>> AVERROR.
>>
>> Strangely, sdp_parse_line is void and it seems like ff_sdp_parse always
>> returns 0, despite the caller (sdp_read_header) attempting to handle any
>> potential returned error codes.
>>
>> On Wed, Feb 26, 2025, 7:52 AM Rashad Tatum <tatum.rashad@gmail.com>
>> wrote:
>>
>>> As a side note,  I think the failure of the rtsp_src memory allocation
>>> should also log an error so that the user is aware of why the rtsp stream
>>> failed to play. However, I think this change should be part of separate
>>> patch series.
>>>
>>> On Wed, Feb 26, 2025, 7:14 AM Rashad Tatum <tatum.rashad@gmail.com>
>>> wrote:
>>>
>>>> Ok. I'll move this and the third patch to one commit/patch and also
>>>> change the condition when rtsp_src can't be allocated to also free
>>>> dest_addr. Good catch.
>>>>
>>>>
>>>> On Wed, Feb 26, 2025, 3:53 AM Andreas Rheinhardt <
>>>> andreas.rheinhardt@outlook.com> wrote:
>>>>
>>>>> Rashad Tatum:
>>>>> > ---
>>>>> >  libavformat/rtsp.c | 2 ++
>>>>> >  1 file changed, 2 insertions(+)
>>>>> >
>>>>> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
>>>>> > index 0c65f8d1a4..ac17717195 100644
>>>>> > --- a/libavformat/rtsp.c
>>>>> > +++ b/libavformat/rtsp.c
>>>>> > @@ -478,6 +478,7 @@ static void sdp_parse_line(AVFormatContext *s,
>>>>> SDPParseState *s1,
>>>>> >                                                sdp_ip_str);
>>>>> >
>>>>> >          }
>>>>> > +        av_freep(&sdp_ip_str);
>>>>> >          break;
>>>>> >      case 's':
>>>>> >          av_dict_set(&s->metadata, "title", p, 0);
>>>>> > @@ -702,6 +703,7 @@ static void sdp_parse_line(AVFormatContext *s,
>>>>> SDPParseState *s1,
>>>>> >                      }
>>>>> >                  }
>>>>> >              }
>>>>> > +            av_freep(&dest_addr);
>>>>> >          } else {
>>>>> >              if (rt->server_type == RTSP_SERVER_WMS)
>>>>> >                  ff_wms_parse_sdp_a_line(s, p);
>>>>>
>>>>> Your first patch introduces a memleak; the stuff you allocated there
>>>>> should be freed in the patch, not fixed up in a later patch.
>>>>> (Also note that there must not be a leak in any error path, which is
>>>>> not
>>>>> true when your patchset is applied: dest_addr leaks when rtsp_src can't
>>>>> be allocated.)
>>>>>
>>>>> - Andreas
>>>>>
>>>>> _______________________________________________
>>>>> 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".

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-02-26 13:19 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-02-26  4:01 [FFmpeg-devel] [PATCH v5 1/3] libavformat/rtsp: Make source specific multicast work for rtsp streams Rashad Tatum
2025-02-26  4:01 ` [FFmpeg-devel] [PATCH v5 2/3] libavformat/rtsp: Free memory allocated for temporary variables while processing sdp info Rashad Tatum
2025-02-26  8:52   ` Andreas Rheinhardt
2025-02-26 12:14     ` Rashad Tatum
2025-02-26 12:52       ` Rashad Tatum
2025-02-26 13:16         ` Rashad Tatum
2025-02-26 13:18           ` Rashad Tatum
2025-02-26 13:19             ` Rashad Tatum
2025-02-26  4:01 ` [FFmpeg-devel] [PATCH v5 3/3] libavformat/rtsp: Fix playback of sdp files from http urls Rashad Tatum

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git