diff mbox series

[FFmpeg-devel,3/4] avcodec/avcodec: Don't reset decoder-fields for encoders when flushing

Message ID AS8PR01MB79446CA20AC237C1CAC32F588FEC9@AS8PR01MB7944.eurprd01.prod.exchangelabs.com
State Accepted
Commit fa3f9f2f6acc48fe6ac6af1ea723066d6851743c
Headers show
Series [FFmpeg-devel,1/4] avcodec/encode: Fix check for encoders impl. encode-simple API | 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

Andreas Rheinhardt April 13, 2022, 2:49 p.m. UTC 
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
Needs to be applied before
https://ffmpeg.org/pipermail/ffmpeg-devel/2022-March/294507.html
or flushing an encoder will segfault.
Btw: All this stuff is unused by subtitle decoders,
so one could condition the else on that and avoid the allocations.

 libavcodec/avcodec.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

Comments

Anton Khirnov April 13, 2022, 3:48 p.m. UTC | #1 
Quoting Andreas Rheinhardt (2022-04-13 16:49:51)
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
> Needs to be applied before
> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-March/294507.html
> or flushing an encoder will segfault.
> Btw: All this stuff is unused by subtitle decoders,
> so one could condition the else on that and avoid the allocations.
> 
>  libavcodec/avcodec.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)
> 
> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
> index ded6b5b307..0d971a61d4 100644
> --- a/libavcodec/avcodec.c
> +++ b/libavcodec/avcodec.c
> @@ -422,6 +422,17 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
>          }
>          if (avci->in_frame)
>              av_frame_unref(avci->in_frame);
> +    } else {
> +        av_packet_unref(avci->last_pkt_props);
> +        while (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) >= 0)
> +            av_packet_unref(avci->last_pkt_props);
> +
> +        av_packet_unref(avci->in_pkt);
> +
> +        avctx->pts_correction_last_pts =
> +        avctx->pts_correction_last_dts = INT64_MIN;
> +
> +        av_bsf_flush(avci->bsf);
>      }
>  
>      avci->draining      = 0;
> @@ -430,22 +441,10 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
>      av_frame_unref(avci->buffer_frame);
>      av_packet_unref(avci->buffer_pkt);
>  
> -    av_packet_unref(avci->last_pkt_props);
> -    while (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) >= 0)
> -        av_packet_unref(avci->last_pkt_props);
> -
> -    av_packet_unref(avci->in_pkt);
> -
>      if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME)
>          ff_thread_flush(avctx);
>      else if (ffcodec(avctx->codec)->flush)
>          ffcodec(avctx->codec)->flush(avctx);
> -
> -    avctx->pts_correction_last_pts =
> -    avctx->pts_correction_last_dts = INT64_MIN;
> -
> -    if (avci->bsf)

did you drop this check on purpose?
Andreas Rheinhardt April 13, 2022, 3:58 p.m. UTC | #2 
Anton Khirnov:
> Quoting Andreas Rheinhardt (2022-04-13 16:49:51)
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>> Needs to be applied before
>> https://ffmpeg.org/pipermail/ffmpeg-devel/2022-March/294507.html
>> or flushing an encoder will segfault.
>> Btw: All this stuff is unused by subtitle decoders,
>> so one could condition the else on that and avoid the allocations.
>>
>>  libavcodec/avcodec.c | 23 +++++++++++------------
>>  1 file changed, 11 insertions(+), 12 deletions(-)
>>
>> diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
>> index ded6b5b307..0d971a61d4 100644
>> --- a/libavcodec/avcodec.c
>> +++ b/libavcodec/avcodec.c
>> @@ -422,6 +422,17 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
>>          }
>>          if (avci->in_frame)
>>              av_frame_unref(avci->in_frame);
>> +    } else {
>> +        av_packet_unref(avci->last_pkt_props);
>> +        while (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) >= 0)
>> +            av_packet_unref(avci->last_pkt_props);
>> +
>> +        av_packet_unref(avci->in_pkt);
>> +
>> +        avctx->pts_correction_last_pts =
>> +        avctx->pts_correction_last_dts = INT64_MIN;
>> +
>> +        av_bsf_flush(avci->bsf);
>>      }
>>  
>>      avci->draining      = 0;
>> @@ -430,22 +441,10 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
>>      av_frame_unref(avci->buffer_frame);
>>      av_packet_unref(avci->buffer_pkt);
>>  
>> -    av_packet_unref(avci->last_pkt_props);
>> -    while (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) >= 0)
>> -        av_packet_unref(avci->last_pkt_props);
>> -
>> -    av_packet_unref(avci->in_pkt);
>> -
>>      if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME)
>>          ff_thread_flush(avctx);
>>      else if (ffcodec(avctx->codec)->flush)
>>          ffcodec(avctx->codec)->flush(avctx);
>> -
>> -    avctx->pts_correction_last_pts =
>> -    avctx->pts_correction_last_dts = INT64_MIN;
>> -
>> -    if (avci->bsf)
> 
> did you drop this check on purpose?
> 

Yes. We currently open bsfs for all decoders (even if it is a null-bsf).

- Andreas
Anton Khirnov April 13, 2022, 4:01 p.m. UTC | #3 
Quoting Andreas Rheinhardt (2022-04-13 17:58:08)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2022-04-13 16:49:51)
> >> -
> >> -    avctx->pts_correction_last_pts =
> >> -    avctx->pts_correction_last_dts = INT64_MIN;
> >> -
> >> -    if (avci->bsf)
> > 
> > did you drop this check on purpose?
> > 
> 
> Yes. We currently open bsfs for all decoders (even if it is a null-bsf).

Then patch lgtm
diff mbox series

Patch

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index ded6b5b307..0d971a61d4 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -422,6 +422,17 @@  void avcodec_flush_buffers(AVCodecContext *avctx)
         }
         if (avci->in_frame)
             av_frame_unref(avci->in_frame);
+    } else {
+        av_packet_unref(avci->last_pkt_props);
+        while (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) >= 0)
+            av_packet_unref(avci->last_pkt_props);
+
+        av_packet_unref(avci->in_pkt);
+
+        avctx->pts_correction_last_pts =
+        avctx->pts_correction_last_dts = INT64_MIN;
+
+        av_bsf_flush(avci->bsf);
     }
 
     avci->draining      = 0;
@@ -430,22 +441,10 @@  void avcodec_flush_buffers(AVCodecContext *avctx)
     av_frame_unref(avci->buffer_frame);
     av_packet_unref(avci->buffer_pkt);
 
-    av_packet_unref(avci->last_pkt_props);
-    while (av_fifo_read(avci->pkt_props, avci->last_pkt_props, 1) >= 0)
-        av_packet_unref(avci->last_pkt_props);
-
-    av_packet_unref(avci->in_pkt);
-
     if (HAVE_THREADS && avctx->active_thread_type & FF_THREAD_FRAME)
         ff_thread_flush(avctx);
     else if (ffcodec(avctx->codec)->flush)
         ffcodec(avctx->codec)->flush(avctx);
-
-    avctx->pts_correction_last_pts =
-    avctx->pts_correction_last_dts = INT64_MIN;
-
-    if (avci->bsf)
-        av_bsf_flush(avci->bsf);
 }
 
 void avsubtitle_free(AVSubtitle *sub)