diff mbox series

[FFmpeg-devel,v3,1/2] avformat/rtsp: load the sdp file with avio_read_to_bprint()

Message ID 1638441459-21819-1-git-send-email-lance.lmwang@gmail.com
State Accepted
Commit 98054e4f018fefb83c1903de99cdd9e9c3394e85
Headers show
Series [FFmpeg-devel,v3,1/2] avformat/rtsp: load the sdp file with avio_read_to_bprint() | expand

Checks

Context Check Description
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished
andriy/make_ppc success Make finished
andriy/make_fate_ppc success Make fate finished

Commit Message

Lance Wang Dec. 2, 2021, 10:37 a.m. UTC
From: Limin Wang <lance.lmwang@gmail.com>

this allows getting rid of the hardcoded max size of SDP.

Reviewed-by: Martin Storsjö <martin@martin.st>
Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
---
 libavformat/rtsp.c | 25 +++++++++----------------
 1 file changed, 9 insertions(+), 16 deletions(-)

Comments

Martin Storsjö Dec. 2, 2021, 11:19 a.m. UTC | #1
On Thu, 2 Dec 2021, lance.lmwang@gmail.com wrote:

> From: Limin Wang <lance.lmwang@gmail.com>
>
> this allows getting rid of the hardcoded max size of SDP.
>
> Reviewed-by: Martin Storsjö <martin@martin.st>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
> libavformat/rtsp.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)

LGTM

// Martin
Michael Niedermayer Dec. 2, 2021, 10:52 p.m. UTC | #2
On Thu, Dec 02, 2021 at 06:37:38PM +0800, lance.lmwang@gmail.com wrote:
> From: Limin Wang <lance.lmwang@gmail.com>
> 
> this allows getting rid of the hardcoded max size of SDP.
> 
> Reviewed-by: Martin Storsjö <martin@martin.st>
> Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> ---
>  libavformat/rtsp.c | 25 +++++++++----------------
>  1 file changed, 9 insertions(+), 16 deletions(-)

This seems to break things

cvlc --play-and-exit ~/videos/stream2962.ts --sout="#rtp{dst=127.0.0.1,port=4414,sdp=rtsp://127.0.0.1:9180/test.sdp, mux=ts}" >  /tmp/log.1 2> /tmp/log.2 & ./ffmpeg -i rtp://127.0.0.1:4414 -t 1 -bitexact -y out2962.avi

before this patch
frame=   14 fps=0.0 q=2.0 Lsize=      38kB time=00:00:01.00 bitrate= 309.2kbits/s speed= 117x    
video:32kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 19.610027%

after:
rtp://127.0.0.1:4414: Invalid argument

[...]
Lance Wang Dec. 3, 2021, 1:16 a.m. UTC | #3
On Thu, Dec 02, 2021 at 11:52:23PM +0100, Michael Niedermayer wrote:
> On Thu, Dec 02, 2021 at 06:37:38PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > this allows getting rid of the hardcoded max size of SDP.
> > 
> > Reviewed-by: Martin Storsjö <martin@martin.st>
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavformat/rtsp.c | 25 +++++++++----------------
> >  1 file changed, 9 insertions(+), 16 deletions(-)
> 
> This seems to break things
> 
> cvlc --play-and-exit ~/videos/stream2962.ts --sout="#rtp{dst=127.0.0.1,port=4414,sdp=rtsp://127.0.0.1:9180/test.sdp, mux=ts}" >  /tmp/log.1 2> /tmp/log.2 & ./ffmpeg -i rtp://127.0.0.1:4414 -t 1 -bitexact -y out2962.avi
> 
> before this patch
> frame=   14 fps=0.0 q=2.0 Lsize=      38kB time=00:00:01.00 bitrate= 309.2kbits/s speed= 117x    
> video:32kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 19.610027%
> 
> after:
> rtp://127.0.0.1:4414: Invalid argument

OK, will try to reproduce it.

> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> What does censorship reveal? It reveals fear. -- Julian Assange



> _______________________________________________
> 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".
Lance Wang Dec. 3, 2021, 1:14 p.m. UTC | #4
On Thu, Dec 02, 2021 at 11:52:23PM +0100, Michael Niedermayer wrote:
> On Thu, Dec 02, 2021 at 06:37:38PM +0800, lance.lmwang@gmail.com wrote:
> > From: Limin Wang <lance.lmwang@gmail.com>
> > 
> > this allows getting rid of the hardcoded max size of SDP.
> > 
> > Reviewed-by: Martin Storsjö <martin@martin.st>
> > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > ---
> >  libavformat/rtsp.c | 25 +++++++++----------------
> >  1 file changed, 9 insertions(+), 16 deletions(-)
> 
> This seems to break things
> 
> cvlc --play-and-exit ~/videos/stream2962.ts --sout="#rtp{dst=127.0.0.1,port=4414,sdp=rtsp://127.0.0.1:9180/test.sdp, mux=ts}" >  /tmp/log.1 2> /tmp/log.2 & ./ffmpeg -i rtp://127.0.0.1:4414 -t 1 -bitexact -y out2962.avi
> 
> before this patch
> frame=   14 fps=0.0 q=2.0 Lsize=      38kB time=00:00:01.00 bitrate= 309.2kbits/s speed= 117x    
> video:32kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 19.610027%
> 
> after:
> rtp://127.0.0.1:4414: Invalid argument

Michael, I have submit one more fix for the break. After apply below patch(patch#1), it'll working without error. Please 
check whether it fixed your issue. 

https://patchwork.ffmpeg.org/project/ffmpeg/patch/1638507433-5005-1-git-send-email-lance.lmwang@gmail.com/

> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> What does censorship reveal? It reveals fear. -- Julian Assange



> _______________________________________________
> 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".
Michael Niedermayer Dec. 3, 2021, 1:50 p.m. UTC | #5
On Fri, Dec 03, 2021 at 09:14:12PM +0800, lance.lmwang@gmail.com wrote:
> On Thu, Dec 02, 2021 at 11:52:23PM +0100, Michael Niedermayer wrote:
> > On Thu, Dec 02, 2021 at 06:37:38PM +0800, lance.lmwang@gmail.com wrote:
> > > From: Limin Wang <lance.lmwang@gmail.com>
> > > 
> > > this allows getting rid of the hardcoded max size of SDP.
> > > 
> > > Reviewed-by: Martin Storsjö <martin@martin.st>
> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > ---
> > >  libavformat/rtsp.c | 25 +++++++++----------------
> > >  1 file changed, 9 insertions(+), 16 deletions(-)
> > 
> > This seems to break things
> > 
> > cvlc --play-and-exit ~/videos/stream2962.ts --sout="#rtp{dst=127.0.0.1,port=4414,sdp=rtsp://127.0.0.1:9180/test.sdp, mux=ts}" >  /tmp/log.1 2> /tmp/log.2 & ./ffmpeg -i rtp://127.0.0.1:4414 -t 1 -bitexact -y out2962.avi
> > 
> > before this patch
> > frame=   14 fps=0.0 q=2.0 Lsize=      38kB time=00:00:01.00 bitrate= 309.2kbits/s speed= 117x    
> > video:32kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 19.610027%
> > 
> > after:
> > rtp://127.0.0.1:4414: Invalid argument
> 
> Michael, I have submit one more fix for the break. After apply below patch(patch#1), it'll working without error. Please 
> check whether it fixed your issue. 
> 
> https://patchwork.ffmpeg.org/project/ffmpeg/patch/1638507433-5005-1-git-send-email-lance.lmwang@gmail.com/

confirmed the failure is fixed

thx

[...]
Lance Wang Dec. 3, 2021, 2:16 p.m. UTC | #6
On Fri, Dec 03, 2021 at 02:50:14PM +0100, Michael Niedermayer wrote:
> On Fri, Dec 03, 2021 at 09:14:12PM +0800, lance.lmwang@gmail.com wrote:
> > On Thu, Dec 02, 2021 at 11:52:23PM +0100, Michael Niedermayer wrote:
> > > On Thu, Dec 02, 2021 at 06:37:38PM +0800, lance.lmwang@gmail.com wrote:
> > > > From: Limin Wang <lance.lmwang@gmail.com>
> > > > 
> > > > this allows getting rid of the hardcoded max size of SDP.
> > > > 
> > > > Reviewed-by: Martin Storsjö <martin@martin.st>
> > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
> > > > ---
> > > >  libavformat/rtsp.c | 25 +++++++++----------------
> > > >  1 file changed, 9 insertions(+), 16 deletions(-)
> > > 
> > > This seems to break things
> > > 
> > > cvlc --play-and-exit ~/videos/stream2962.ts --sout="#rtp{dst=127.0.0.1,port=4414,sdp=rtsp://127.0.0.1:9180/test.sdp, mux=ts}" >  /tmp/log.1 2> /tmp/log.2 & ./ffmpeg -i rtp://127.0.0.1:4414 -t 1 -bitexact -y out2962.avi
> > > 
> > > before this patch
> > > frame=   14 fps=0.0 q=2.0 Lsize=      38kB time=00:00:01.00 bitrate= 309.2kbits/s speed= 117x    
> > > video:32kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 19.610027%
> > > 
> > > after:
> > > rtp://127.0.0.1:4414: Invalid argument
> > 
> > Michael, I have submit one more fix for the break. After apply below patch(patch#1), it'll working without error. Please 
> > check whether it fixed your issue. 
> > 
> > https://patchwork.ffmpeg.org/project/ffmpeg/patch/1638507433-5005-1-git-send-email-lance.lmwang@gmail.com/
> 
> confirmed the failure is fixed

Thanks for the confirmed, then I'll applied the patch first before the two patch applied.

> 
> thx
> 
> [...]
> -- 
> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
> 
> The worst form of inequality is to try to make unequal things equal.
> -- Aristotle



> _______________________________________________
> 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".
diff mbox series

Patch

diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index e6a4993..17ffac2 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -2373,9 +2373,9 @@  static int sdp_read_header(AVFormatContext *s)
 {
     RTSPState *rt = s->priv_data;
     RTSPStream *rtsp_st;
-    int size, i, err;
-    char *content;
+    int i, err;
     char url[MAX_URL_SIZE];
+    AVBPrint bp;
 
     if (!ff_network_init())
         return AVERROR(EIO);
@@ -2386,22 +2386,15 @@  static int sdp_read_header(AVFormatContext *s)
         rt->lower_transport = RTSP_LOWER_TRANSPORT_CUSTOM;
 
     /* read the whole sdp file */
-    /* XXX: better loading */
-    content = av_malloc(SDP_MAX_SIZE);
-    if (!content) {
+    av_bprint_init(&bp, 0, AV_BPRINT_SIZE_UNLIMITED);
+    err = avio_read_to_bprint(s->pb, &bp, INT_MAX);
+    if (err < 0 ) {
         ff_network_close();
-        return AVERROR(ENOMEM);
+        av_bprint_finalize(&bp, NULL);
+        return err;
     }
-    size = avio_read(s->pb, content, SDP_MAX_SIZE - 1);
-    if (size <= 0) {
-        av_free(content);
-        ff_network_close();
-        return AVERROR_INVALIDDATA;
-    }
-    content[size] ='\0';
-
-    err = ff_sdp_parse(s, content);
-    av_freep(&content);
+    err = ff_sdp_parse(s, bp.str);
+    av_bprint_finalize(&bp, NULL);
     if (err) goto fail;
 
     /* open each RTP stream */