diff mbox series

[FFmpeg-devel] avformat/rtsp: don't overwrite session control uri

Message ID 20201010232218.295220-1-andriy.gelman@gmail.com
State Accepted
Commit 71ce5c32f0bfcbe053bc9110923bbe7479c358e3
Headers show
Series [FFmpeg-devel] avformat/rtsp: don't overwrite session control uri | expand

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make warning Make failed

Commit Message

Andriy Gelman Oct. 10, 2020, 11:22 p.m. UTC
From: Andriy Gelman <andriy.gelman@gmail.com>

Fixes #1941

Currently the session control uri gets overwritten by the media's uri
when mpegts is signalled in the media description. This happens because
s->nb_streams doesn't count mpegts which is instead part of the private
context in RTSPStream.

Instead use rt->nb_rtsp_streams which counts all of the media streams
signalled in the sdp.

This solution was originally proposed by user "tpol" on trac:
https://trac.ffmpeg.org/ticket/1941

Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
---
 libavformat/rtsp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Andriy Gelman Oct. 21, 2020, 5:39 p.m. UTC | #1
On Sat, 10. Oct 19:22, Andriy Gelman wrote:
> From: Andriy Gelman <andriy.gelman@gmail.com>
> 
> Fixes #1941
> 
> Currently the session control uri gets overwritten by the media's uri
> when mpegts is signalled in the media description. This happens because
> s->nb_streams doesn't count mpegts which is instead part of the private
> context in RTSPStream.
> 
> Instead use rt->nb_rtsp_streams which counts all of the media streams
> signalled in the sdp.
> 
> This solution was originally proposed by user "tpol" on trac:
> https://trac.ffmpeg.org/ticket/1941
> 
> Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> ---
>  libavformat/rtsp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index e9fca034b4..4d5459ac41 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -542,7 +542,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
>          break;
>      case 'a':
>          if (av_strstart(p, "control:", &p)) {
> -            if (s->nb_streams == 0) {
> +            if (rt->nb_rtsp_streams == 0) {
>                  if (!strncmp(p, "rtsp://", 7))
>                      av_strlcpy(rt->control_uri, p,
>                                 sizeof(rt->control_uri));
> -- 
> 2.28.0
> 

ping
Andriy Gelman Oct. 31, 2020, 2:16 p.m. UTC | #2
On Wed, 21. Oct 13:39, Andriy Gelman wrote:
> On Sat, 10. Oct 19:22, Andriy Gelman wrote:
> > From: Andriy Gelman <andriy.gelman@gmail.com>
> > 
> > Fixes #1941
> > 
> > Currently the session control uri gets overwritten by the media's uri
> > when mpegts is signalled in the media description. This happens because
> > s->nb_streams doesn't count mpegts which is instead part of the private
> > context in RTSPStream.
> > 
> > Instead use rt->nb_rtsp_streams which counts all of the media streams
> > signalled in the sdp.
> > 
> > This solution was originally proposed by user "tpol" on trac:
> > https://trac.ffmpeg.org/ticket/1941
> > 
> > Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> > ---
> >  libavformat/rtsp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index e9fca034b4..4d5459ac41 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -542,7 +542,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
> >          break;
> >      case 'a':
> >          if (av_strstart(p, "control:", &p)) {
> > -            if (s->nb_streams == 0) {
> > +            if (rt->nb_rtsp_streams == 0) {
> >                  if (!strncmp(p, "rtsp://", 7))
> >                      av_strlcpy(rt->control_uri, p,
> >                                 sizeof(rt->control_uri));
> > -- 
> > 2.28.0
> > 
> 
> ping
> 

ping
Will apply this Monday if no one objects.
Andriy Gelman Jan. 17, 2021, 10:53 p.m. UTC | #3
On Sat, 31. Oct 10:16, Andriy Gelman wrote:
> On Wed, 21. Oct 13:39, Andriy Gelman wrote:
> > On Sat, 10. Oct 19:22, Andriy Gelman wrote:
> > > From: Andriy Gelman <andriy.gelman@gmail.com>
> > > 
> > > Fixes #1941
> > > 
> > > Currently the session control uri gets overwritten by the media's uri
> > > when mpegts is signalled in the media description. This happens because
> > > s->nb_streams doesn't count mpegts which is instead part of the private
> > > context in RTSPStream.
> > > 
> > > Instead use rt->nb_rtsp_streams which counts all of the media streams
> > > signalled in the sdp.
> > > 
> > > This solution was originally proposed by user "tpol" on trac:
> > > https://trac.ffmpeg.org/ticket/1941
> > > 
> > > Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> > > ---
> > >  libavformat/rtsp.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > > index e9fca034b4..4d5459ac41 100644
> > > --- a/libavformat/rtsp.c
> > > +++ b/libavformat/rtsp.c
> > > @@ -542,7 +542,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
> > >          break;
> > >      case 'a':
> > >          if (av_strstart(p, "control:", &p)) {
> > > -            if (s->nb_streams == 0) {
> > > +            if (rt->nb_rtsp_streams == 0) {
> > >                  if (!strncmp(p, "rtsp://", 7))
> > >                      av_strlcpy(rt->control_uri, p,
> > >                                 sizeof(rt->control_uri));
> > > -- 
> > > 2.28.0
> > > 
> > 
> > ping
> > 
> 
> ping
> Will apply this Monday if no one objects.

I haven't applied this patch yet because I had some problems testing with vlc.
vlc would close the socket connection after ~10 seconds, even though a 60second
timeout was advertised.

This was fairly recently fixed in vlc:
38d214bc4f2ef68dfe6859383e12a20ae91e62de
ce3269d4a20aba2f87fc3e512131cb31b9561b3e

So I'll go ahead and push.
Carl Eugen Hoyos Jan. 17, 2021, 11:49 p.m. UTC | #4
Am So., 11. Okt. 2020 um 01:54 Uhr schrieb Andriy Gelman
<andriy.gelman@gmail.com>:
>
> From: Andriy Gelman <andriy.gelman@gmail.com>
>
> Fixes #1941
>
> Currently the session control uri gets overwritten by the media's uri
> when mpegts is signalled in the media description. This happens because
> s->nb_streams doesn't count mpegts which is instead part of the private
> context in RTSPStream.
>
> Instead use rt->nb_rtsp_streams which counts all of the media streams
> signalled in the sdp.
>
> This solution was originally proposed by user "tpol" on trac:
> https://trac.ffmpeg.org/ticket/1941
>
> Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> ---
>  libavformat/rtsp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> index e9fca034b4..4d5459ac41 100644
> --- a/libavformat/rtsp.c
> +++ b/libavformat/rtsp.c
> @@ -542,7 +542,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
>          break;
>      case 'a':
>          if (av_strstart(p, "control:", &p)) {
> -            if (s->nb_streams == 0) {
> +            if (rt->nb_rtsp_streams == 0) {
>                  if (!strncmp(p, "rtsp://", 7))
>                      av_strlcpy(rt->control_uri, p,
>                                 sizeof(rt->control_uri));

Wasn't this patch written by tpol?

Carl Eugen
Andriy Gelman Jan. 18, 2021, 12:55 a.m. UTC | #5
On Mon, 18. Jan 00:49, Carl Eugen Hoyos wrote:
> Am So., 11. Okt. 2020 um 01:54 Uhr schrieb Andriy Gelman
> <andriy.gelman@gmail.com>:
> >
> > From: Andriy Gelman <andriy.gelman@gmail.com>
> >
> > Fixes #1941
> >
> > Currently the session control uri gets overwritten by the media's uri
> > when mpegts is signalled in the media description. This happens because
> > s->nb_streams doesn't count mpegts which is instead part of the private
> > context in RTSPStream.
> >
> > Instead use rt->nb_rtsp_streams which counts all of the media streams
> > signalled in the sdp.
> >
> > This solution was originally proposed by user "tpol" on trac:
> > https://trac.ffmpeg.org/ticket/1941
> >
> > Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> > ---
> >  libavformat/rtsp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index e9fca034b4..4d5459ac41 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -542,7 +542,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
> >          break;
> >      case 'a':
> >          if (av_strstart(p, "control:", &p)) {
> > -            if (s->nb_streams == 0) {
> > +            if (rt->nb_rtsp_streams == 0) {
> >                  if (!strncmp(p, "rtsp://", 7))
> >                      av_strlcpy(rt->control_uri, p,
> >                                 sizeof(rt->control_uri));

> 
> Wasn't this patch written by tpol?

I did say in the commit message that it was proposed by tpol from trac.
Afaik the patch wasn't sent to the ML, so there's no email for author.

What do you suggest?

btw the updated first part of the commit message is:

"Currently the session control uri gets overwritten by the media's uri
when mpegts is signalled in the media description. This happens because
s->nb_streams doesn't count mpegts (AVStreams are added when parsing
mpegts packets). Instead use rt->nb_rtsp_streams, which also counts
mpegts."
Andriy Gelman Jan. 18, 2021, 2:43 a.m. UTC | #6
On Sun, 17. Jan 19:55, Andriy Gelman wrote:
> On Mon, 18. Jan 00:49, Carl Eugen Hoyos wrote:
> > Am So., 11. Okt. 2020 um 01:54 Uhr schrieb Andriy Gelman
> > <andriy.gelman@gmail.com>:
> > >
> > > From: Andriy Gelman <andriy.gelman@gmail.com>
> > >
> > > Fixes #1941
> > >
> > > Currently the session control uri gets overwritten by the media's uri
> > > when mpegts is signalled in the media description. This happens because
> > > s->nb_streams doesn't count mpegts which is instead part of the private
> > > context in RTSPStream.
> > >
> > > Instead use rt->nb_rtsp_streams which counts all of the media streams
> > > signalled in the sdp.
> > >
> > > This solution was originally proposed by user "tpol" on trac:
> > > https://trac.ffmpeg.org/ticket/1941
> > >
> > > Signed-off-by: Andriy Gelman <andriy.gelman@gmail.com>
> > > ---
> > >  libavformat/rtsp.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > > index e9fca034b4..4d5459ac41 100644
> > > --- a/libavformat/rtsp.c
> > > +++ b/libavformat/rtsp.c
> > > @@ -542,7 +542,7 @@ static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
> > >          break;
> > >      case 'a':
> > >          if (av_strstart(p, "control:", &p)) {
> > > -            if (s->nb_streams == 0) {
> > > +            if (rt->nb_rtsp_streams == 0) {
> > >                  if (!strncmp(p, "rtsp://", 7))
> > >                      av_strlcpy(rt->control_uri, p,
> > >                                 sizeof(rt->control_uri));
> 

> > 
> > Wasn't this patch written by tpol?
> 
> I did say in the commit message that it was proposed by tpol from trac.
> Afaik the patch wasn't sent to the ML, so there's no email for author.
> 
> What do you suggest?
> 

I saw there are other commits with empty emails. I changed the author 
to tpol and pushed.
diff mbox series

Patch

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index e9fca034b4..4d5459ac41 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -542,7 +542,7 @@  static void sdp_parse_line(AVFormatContext *s, SDPParseState *s1,
         break;
     case 'a':
         if (av_strstart(p, "control:", &p)) {
-            if (s->nb_streams == 0) {
+            if (rt->nb_rtsp_streams == 0) {
                 if (!strncmp(p, "rtsp://", 7))
                     av_strlcpy(rt->control_uri, p,
                                sizeof(rt->control_uri));