Message ID | 62f9da71ba5d0a043692ef44102be640f9021c21.1587126983.git.pross@xvid.org |
---|---|
State | Superseded |
Headers | show |
Series | [FFmpeg-devel] ffplay: do not segfault on stream_open internal error | expand |
Context | Check | Description |
---|---|---|
andriy/default | pending | |
andriy/make | success | Make finished |
andriy/make_fate | success | Make fate finished |
On Fri, 17 Apr 2020, Peter Ross wrote: > is->ic is assigned by read_thread, but this may not have happened yet > when stream_open fails. I think the idea was that audio_stream/video_stream/subtitle_stream should be -1 if no stream is opened. But that is not true, because it is only initialized to -1 in read_thread. So I prefer we move the initialization to -1 from read_thread to stream_open, and we can also init last_*_stream variables there as well. Thanks, Marton > --- > fftools/ffplay.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fftools/ffplay.c b/fftools/ffplay.c > index 1beec54293..5530d2a396 100644 > --- a/fftools/ffplay.c > +++ b/fftools/ffplay.c > @@ -1210,7 +1210,7 @@ static void stream_component_close(VideoState *is, int stream_index) > AVFormatContext *ic = is->ic; > AVCodecParameters *codecpar; > > - if (stream_index < 0 || stream_index >= ic->nb_streams) > + if (!ic || stream_index < 0 || stream_index >= ic->nb_streams) > return; > codecpar = ic->streams[stream_index]->codecpar; > > -- > 2.20.1 > > -- Peter > (A907 E02F A6E5 0CD2 34CD 20D2 6760 79C5 AC40 DD6B) >
Signed-off-by: Peter Ross <pross@xvid.org> Reviewed-by: Marton Balint <cus@passwd.hu> --- Great suggestion. I will apply in a few days if no objections. fftools/ffplay.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fftools/ffplay.c b/fftools/ffplay.c index 1beec54293..d673b8049a 100644 --- a/fftools/ffplay.c +++ b/fftools/ffplay.c @@ -2775,9 +2775,6 @@ static int read_thread(void *arg) } memset(st_index, -1, sizeof(st_index)); - is->last_video_stream = is->video_stream = -1; - is->last_audio_stream = is->audio_stream = -1; - is->last_subtitle_stream = is->subtitle_stream = -1; is->eof = 0; ic = avformat_alloc_context(); @@ -3083,6 +3080,9 @@ static VideoState *stream_open(const char *filename, AVInputFormat *iformat) is = av_mallocz(sizeof(VideoState)); if (!is) return NULL; + is->last_video_stream = is->video_stream = -1; + is->last_audio_stream = is->audio_stream = -1; + is->last_subtitle_stream = is->subtitle_stream = -1; is->filename = av_strdup(filename); if (!is->filename) goto fail;
On Sat, 18 Apr 2020, Peter Ross wrote: > Signed-off-by: Peter Ross <pross@xvid.org> > Reviewed-by: Marton Balint <cus@passwd.hu> > --- > > Great suggestion. I will apply in a few days if no objections. > > fftools/ffplay.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/fftools/ffplay.c b/fftools/ffplay.c > index 1beec54293..d673b8049a 100644 > --- a/fftools/ffplay.c > +++ b/fftools/ffplay.c > @@ -2775,9 +2775,6 @@ static int read_thread(void *arg) > } > > memset(st_index, -1, sizeof(st_index)); > - is->last_video_stream = is->video_stream = -1; > - is->last_audio_stream = is->audio_stream = -1; > - is->last_subtitle_stream = is->subtitle_stream = -1; > is->eof = 0; > > ic = avformat_alloc_context(); > @@ -3083,6 +3080,9 @@ static VideoState *stream_open(const char *filename, AVInputFormat *iformat) > is = av_mallocz(sizeof(VideoState)); > if (!is) > return NULL; > + is->last_video_stream = is->video_stream = -1; > + is->last_audio_stream = is->audio_stream = -1; > + is->last_subtitle_stream = is->subtitle_stream = -1; > is->filename = av_strdup(filename); > if (!is->filename) > goto fail; LGTM, thanks. Marton
diff --git a/fftools/ffplay.c b/fftools/ffplay.c index 1beec54293..5530d2a396 100644 --- a/fftools/ffplay.c +++ b/fftools/ffplay.c @@ -1210,7 +1210,7 @@ static void stream_component_close(VideoState *is, int stream_index) AVFormatContext *ic = is->ic; AVCodecParameters *codecpar; - if (stream_index < 0 || stream_index >= ic->nb_streams) + if (!ic || stream_index < 0 || stream_index >= ic->nb_streams) return; codecpar = ic->streams[stream_index]->codecpar;