[FFmpeg-devel] [PATCH 8/8] avcodec/decode: copy the output parameters from the last bsf in the chain back to the AVCodecContext

James Almer jamrial at gmail.com
Sat Jul 28 19:25:36 EEST 2018


On 7/28/2018 4:09 AM, Michael Niedermayer wrote:
> On Fri, Jul 27, 2018 at 11:11:47PM -0300, James Almer wrote:
>> On 7/27/2018 10:58 PM, Michael Niedermayer wrote:
>>> On Fri, Jul 27, 2018 at 11:57:49AM -0300, James Almer wrote:
>>>> Certain AVCodecParameters, like the contents of the extradata, may be changed
>>>> by the init() function of any of the bitstream filters in the chain.
>>>>
>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>> ---
>>>> Now it's not going to be called after the codec has been opened.
>>>>
>>>>  libavcodec/decode.c | 4 ++++
>>>>  1 file changed, 4 insertions(+)
>>>
>>> This breaks:
>>> ffmpeg -i https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png -bitexact -pix_fmt rgba -f framecrc -
>>
>> Is any other decoder failing the same way? Because the apng decoder
>> threading code may be faulty otherwise. Plenty of avctx fields are
>> changed after ff_thread_init() is called within avcodec_open2(). There
>> should not be a race at this point.
> 
> I found a failure with mpeg4 (with bframes) decoding + pcm_alaw from mkv but it
> does not want to reproduce. The slightest change i do makes this not happen
> even just duplicating a command line parameter (which should have no effect)
> simply adding the -threads parameter to it independant of value makes it go away
> too
> 
> 
> in the png case
> this hits teh issue:
> -threads 2 -i https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png  -f framecrc -
> 
> this does not:
> -threads 1 -i https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png  -f framecrc -
> 
> also odly the bitexact flag made a differnce in how it fails outside valgrind
> last i tried. (doesnt make a difference in valgrind it seems)

A solution may be moving the ff_decode_bsfs_init call in patch 7/8 right
above the call to ff_thread_init (See attachment), hopefully preventing
this race once this patch is applied afterwards, but it will result in
the bsfs initialized before the decoder, and some of the avctx fields
that are changed later in avcodec_open2 like channels and bit_rate not
being reflected during said bsfs initialization.
I can't say if the former is correct or ideal, but for now the latter
would not be an issue. I don't know what may happen if we were to
autoinsert a filter that does care about such fields in the future, though.

If the above change doesn't solve it, or is not ideal, then this patch
8/8 can be dropped or postponed, and the rest of the set pushed without it.
-------------- next part --------------
From f0e852857b3d218ca5e483ac47df8cb58ff2a362 Mon Sep 17 00:00:00 2001
From: James Almer <jamrial at gmail.com>
Date: Thu, 26 Jul 2018 20:42:27 -0300
Subject: [PATCH 7/8 v2] avcodec/decode: flush the internal bsfs instead of constantly
 reinitalizing them

Initialize the bsfs once when opening the codec and uninitialize them once when
closing it, instead of at every codec flush/seek.

Signed-off-by: James Almer <jamrial at gmail.com>
---
 libavcodec/decode.c | 20 ++++++++++----------
 libavcodec/decode.h |  2 ++
 libavcodec/utils.c  |  7 +++++++
 3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index db364ca700..2e82f6b506 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -182,7 +182,7 @@ static int unrefcount_frame(AVCodecInternal *avci, AVFrame *frame)
     return 0;
 }
 
-static int bsfs_init(AVCodecContext *avctx)
+int ff_decode_bsfs_init(AVCodecContext *avctx)
 {
     AVCodecInternal *avci = avctx->internal;
     DecodeFilterContext *s = &avci->filter;
@@ -688,10 +688,6 @@ int attribute_align_arg avcodec_send_packet(AVCodecContext *avctx, const AVPacke
     if (avpkt && !avpkt->size && avpkt->data)
         return AVERROR(EINVAL);
 
-    ret = bsfs_init(avctx);
-    if (ret < 0)
-        return ret;
-
     av_packet_unref(avci->buffer_pkt);
     if (avpkt && (avpkt->data || avpkt->side_data_elems)) {
         ret = av_packet_ref(avci->buffer_pkt, avpkt);
@@ -751,10 +747,6 @@ int attribute_align_arg avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
     if (!avcodec_is_open(avctx) || !av_codec_is_decoder(avctx->codec))
         return AVERROR(EINVAL);
 
-    ret = bsfs_init(avctx);
-    if (ret < 0)
-        return ret;
-
     if (avci->buffer_frame->buf[0]) {
         av_frame_move_ref(frame, avci->buffer_frame);
     } else {
@@ -1978,6 +1970,14 @@ int ff_reget_buffer(AVCodecContext *avctx, AVFrame *frame)
     return ret;
 }
 
+static void bsfs_flush(AVCodecContext *avctx)
+{
+    DecodeFilterContext *s = &avctx->internal->filter;
+
+    for (int i = 0; i < s->nb_bsfs; i++)
+        av_bsf_flush(s->bsfs[i]);
+}
+
 void avcodec_flush_buffers(AVCodecContext *avctx)
 {
     avctx->internal->draining      = 0;
@@ -1998,7 +1998,7 @@ void avcodec_flush_buffers(AVCodecContext *avctx)
     avctx->pts_correction_last_pts =
     avctx->pts_correction_last_dts = INT64_MIN;
 
-    ff_decode_bsfs_uninit(avctx);
+    bsfs_flush(avctx);
 
     if (!avctx->refcounted_frames)
         av_frame_unref(avctx->internal->to_free);
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index 15271c529a..c3e0e82f4c 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -64,6 +64,8 @@ typedef struct FrameDecodeData {
  */
 int ff_decode_get_packet(AVCodecContext *avctx, AVPacket *pkt);
 
+int ff_decode_bsfs_init(AVCodecContext *avctx);
+
 void ff_decode_bsfs_uninit(AVCodecContext *avctx);
 
 /**
diff --git a/libavcodec/utils.c b/libavcodec/utils.c
index 4f9a2b76ef..285bfdbc63 100644
--- a/libavcodec/utils.c
+++ b/libavcodec/utils.c
@@ -727,6 +727,12 @@ int attribute_align_arg avcodec_open2(AVCodecContext *avctx, const AVCodec *code
             goto free_and_end;
     }
 
+    if (av_codec_is_decoder(avctx->codec)) {
+        ret = ff_decode_bsfs_init(avctx);
+        if (ret < 0)
+            goto free_and_end;
+    }
+
     if (HAVE_THREADS
         && !(avctx->internal->frame_thread_encoder && (avctx->active_thread_type&FF_THREAD_FRAME))) {
         ret = ff_thread_init(avctx);
@@ -1032,6 +1038,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
         av_packet_free(&avctx->internal->last_pkt_props);
 
         av_packet_free(&avctx->internal->ds.in_pkt);
+        ff_decode_bsfs_uninit(avctx);
 
         av_freep(&avctx->internal->pool);
     }
-- 
2.18.0



More information about the ffmpeg-devel mailing list