[FFmpeg-devel] [PATCH v2] avformat/rtsp: load the sdp file with avio_read_to_bprint()

[lance.lmwang at gmail.com]

On Thu, Dec 02, 2021 at 10:04:37AM +0200, Martin Storsjö wrote:
> On Thu, 2 Dec 2021, lance.lmwang at gmail.com wrote:
> 
> > From: Limin Wang <lance.lmwang at gmail.com>
> > 
> > Signed-off-by: Limin Wang <lance.lmwang at gmail.com>
> > ---
> > libavformat/rtsp.c | 23 +++++++++--------------
> > 1 file changed, 9 insertions(+), 14 deletions(-)
> > 
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index e6a4993..249a019 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -2373,9 +2373,10 @@ static int sdp_read_header(AVFormatContext *s)
> > {
> >     RTSPState *rt = s->priv_data;
> >     RTSPStream *rtsp_st;
> > -    int size, i, err;
> > +    int i, err;
> >     char *content;
> >     char url[MAX_URL_SIZE];
> > +    AVBPrint bp;
> > 
> >     if (!ff_network_init())
> >         return AVERROR(EIO);
> > @@ -2386,22 +2387,16 @@ 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';
> > -
> > +    content = bp.str;
> >     err = ff_sdp_parse(s, content);
> 
> The content variable could be eliminated here too (to avoid leaving a
> dangling pointer local variable after freeing it).

OK, will remove content.

> 
> > -    av_freep(&content);
> > +    av_bprint_finalize(&bp, NULL);
> >     if (err) goto fail;
> > 
> >     /* open each RTP stream */
> > -- 
> > 1.8.3.1
> 
> This patch seems ok to me - maybe it'd be good to mention in the commit
> message that this allows getting rid of the hardcoded max size here, to
> explain the "why" for the patch, not only the "what"?
OK, will add below comment message:
this allows getting rid of the hardcoded max size of SDP.

> 
> Would you be interested in getting rid of the same hardcoded max size in
> rtspdec.c too?

OK, will submit a patch for that.

> 
> // Martin
> 

-- 
Thanks,
Limin Wang