diff mbox series

[FFmpeg-devel,5/8] fftools/ffmpeg_demux: only allocate the decoder when actually decoding

Message ID 20240105164251.28935-5-anton@khirnov.net
State Accepted
Commit 50448ca290811fe76192e89000b279f1361033f5
Headers show
Series [FFmpeg-devel,1/8] fftools/ffmpeg_demux: replace abort() by av_assert0(0) | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Anton Khirnov Jan. 5, 2024, 4:42 p.m. UTC 
It is not needed otherwise.
---
 fftools/ffmpeg_demux.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Stefano Sabatini Jan. 6, 2024, 11:34 a.m. UTC | #1 
On date Friday 2024-01-05 17:42:48 +0100, Anton Khirnov wrote:
> It is not needed otherwise.
> ---
>  fftools/ffmpeg_demux.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> index 892094c512..c51140b1c5 100644
> --- a/fftools/ffmpeg_demux.c
> +++ b/fftools/ffmpeg_demux.c
> @@ -782,6 +782,16 @@ static int ist_use(InputStream *ist, int decoding_needed)
>      if (decoding_needed && ds->sch_idx_dec < 0) {
>          int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO;
>  
> +        ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> +        if (!ist->dec_ctx)
> +            return AVERROR(ENOMEM);
> +
> +        ret = avcodec_parameters_to_context(ist->dec_ctx, ist->par);
> +        if (ret < 0) {

> +            av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");

unrelated, here and below: might be useful to notify the failing
stream identifier, assuming it is not already shown in the log

> +            return ret;
> +        }
> +
>          ret = sch_add_dec(d->sch, decoder_thread, ist, d->loop && is_audio);
>          if (ret < 0)
>              return ret;
> @@ -1215,23 +1225,13 @@ static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
>      default: av_assert0(0);
>      }
>  
> -    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> -    if (!ist->dec_ctx)
> -        return AVERROR(ENOMEM);
> -
> -    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
> -    if (ret < 0) {
> -        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> -        return ret;
> -    }
> -
>      ist->par = avcodec_parameters_alloc();
>      if (!ist->par)
>          return AVERROR(ENOMEM);
>  
> -    ret = avcodec_parameters_from_context(ist->par, ist->dec_ctx);
> +    ret = avcodec_parameters_copy(ist->par, par);
>      if (ret < 0) {
> -        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> +        av_log(ist, AV_LOG_ERROR, "Error exporting stream parameters.\n");
>          return ret;
>      }

LGTM anyway.
Anton Khirnov Jan. 16, 2024, 7:50 p.m. UTC | #2 
Quoting Stefano Sabatini (2024-01-06 12:34:53)
> On date Friday 2024-01-05 17:42:48 +0100, Anton Khirnov wrote:
> > It is not needed otherwise.
> > ---
> >  fftools/ffmpeg_demux.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
> > index 892094c512..c51140b1c5 100644
> > --- a/fftools/ffmpeg_demux.c
> > +++ b/fftools/ffmpeg_demux.c
> > @@ -782,6 +782,16 @@ static int ist_use(InputStream *ist, int decoding_needed)
> >      if (decoding_needed && ds->sch_idx_dec < 0) {
> >          int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO;
> >  
> > +        ist->dec_ctx = avcodec_alloc_context3(ist->dec);
> > +        if (!ist->dec_ctx)
> > +            return AVERROR(ENOMEM);
> > +
> > +        ret = avcodec_parameters_to_context(ist->dec_ctx, ist->par);
> > +        if (ret < 0) {
> 
> > +            av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
> 
> unrelated, here and below: might be useful to notify the failing
> stream identifier, assuming it is not already shown in the log

It is, logging to ist prints something like
[aist#0:0/pcm_s16le @ 0x563a40304b40] ....
diff mbox series

Patch

diff --git a/fftools/ffmpeg_demux.c b/fftools/ffmpeg_demux.c
index 892094c512..c51140b1c5 100644
--- a/fftools/ffmpeg_demux.c
+++ b/fftools/ffmpeg_demux.c
@@ -782,6 +782,16 @@  static int ist_use(InputStream *ist, int decoding_needed)
     if (decoding_needed && ds->sch_idx_dec < 0) {
         int is_audio = ist->st->codecpar->codec_type == AVMEDIA_TYPE_AUDIO;
 
+        ist->dec_ctx = avcodec_alloc_context3(ist->dec);
+        if (!ist->dec_ctx)
+            return AVERROR(ENOMEM);
+
+        ret = avcodec_parameters_to_context(ist->dec_ctx, ist->par);
+        if (ret < 0) {
+            av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
+            return ret;
+        }
+
         ret = sch_add_dec(d->sch, decoder_thread, ist, d->loop && is_audio);
         if (ret < 0)
             return ret;
@@ -1215,23 +1225,13 @@  static int ist_add(const OptionsContext *o, Demuxer *d, AVStream *st)
     default: av_assert0(0);
     }
 
-    ist->dec_ctx = avcodec_alloc_context3(ist->dec);
-    if (!ist->dec_ctx)
-        return AVERROR(ENOMEM);
-
-    ret = avcodec_parameters_to_context(ist->dec_ctx, par);
-    if (ret < 0) {
-        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
-        return ret;
-    }
-
     ist->par = avcodec_parameters_alloc();
     if (!ist->par)
         return AVERROR(ENOMEM);
 
-    ret = avcodec_parameters_from_context(ist->par, ist->dec_ctx);
+    ret = avcodec_parameters_copy(ist->par, par);
     if (ret < 0) {
-        av_log(ist, AV_LOG_ERROR, "Error initializing the decoder context.\n");
+        av_log(ist, AV_LOG_ERROR, "Error exporting stream parameters.\n");
         return ret;
     }