[FFmpeg-devel] ffplay: do not segfault on stream_open internal error

is->ic is assigned by read_thread, but this may not have happened yet
when stream_open fails.
---
 fftools/ffplay.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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