Hi Martin,
Thank you for reviewing the set.
On Fri, 20. Nov 10:33, Martin Storsjö wrote:
> On Mon, 12 Oct 2020, Andriy Gelman wrote:
>
> > From: Andriy Gelman <andriy.gel...@gmail.com>
> >
> > In sdp_read_header() some ff_network_close() calls were missed.
> >
> > Also in rtp_read_header() update comment to explain why a single
> > call to ff_network_close() is enough to cover all cases even if
> > sdp_read_header() returns an error.
> >
> > Signed-off-by: Andriy Gelman <andriy.gel...@gmail.com>
> > ---
> > libavformat/rtsp.c | 9 +++++++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
> > index cb9fc31166..a8c7ec4a46 100644
> > --- a/libavformat/rtsp.c
> > +++ b/libavformat/rtsp.c
> > @@ -2347,11 +2347,14 @@ static int sdp_read_header(AVFormatContext *s)
> > /* read the whole sdp file */
> > /* XXX: better loading */
> > content = av_malloc(SDP_MAX_SIZE);
> > - if (!content)
> > + if (!content) {
> > + ff_network_close();
> > return AVERROR(ENOMEM);
> > + }
> > 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';
> > @@ -2550,7 +2553,9 @@ static int rtp_read_header(AVFormatContext *s)
> > ffio_init_context(&pb, sdp.str, sdp.len, 0, NULL, NULL, NULL, NULL);
> > s->pb = &pb;
> >
> > - /* sdp_read_header initializes this again */
> > + /* if sdp_read_header() fails then following ff_network_close()
> > cancels out */
> > + /* ff_network_init() at the start of this function. Otherwise it
> > cancels out */
> > + /* ff_network_init() inside sdp_read_header() */
> > ff_network_close();
> >
> > rt->media_type_mask = (1 << (AVMEDIA_TYPE_SUBTITLE+1)) - 1;
> > --
> > 2.28.0
>
> Ok
>
> Unrelatedly, as we seem to have a problem with
> ff_network_init()/ff_network_close() often being mismatched and not handled
> correctly, do you think it'd be good as a development aid, to make certain
> network functions (ff_url_join, the low level tcp protocol etc) error out on
> unix systems as well, if ff_network_init() has been missed? And same for
> counting the calls to init/close to make sure they're properly paired.
> Otherwise these issues only hit people on windows, where the code probably
> is tested much less.
>
> The problem, I guess, is finding the right level of obnoxity for the error.
> We probably don't want such an issue to break code that otherwise has run
> fine in production on unix platforms, even though it points out a
> portability problem. And as far as I know, most developers don't really
> build/test ffmpeg in debug mode anyway, so if the checks are hidden in such
> a mode, they won't help developers notice the issues either. And we don't
> have much fate coverage for the protocols.
I think it would be a useful tool. But finding the right balance seems tricky..
I'll try to think more about the problem and propose something that's not too
intrusive.
--
Andriy
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
