[FFmpeg-devel] [PATCH 1/6] avcodec/apng: Remove threading support

[Donny Yang]

On 3 June 2015 at 03:31, Paul B Mahol <onemda at gmail.com> wrote:

> Dana 2. 6. 2015. 17:49 osoba "Donny Yang" <work at kota.moe> napisala je:
> >
> > Each frame depends on the previous frame any way, and it will
> > cause bugs with frame disposal
> >
> > Signed-off-by: Donny Yang <work at kota.moe>
> > ---
> >  libavcodec/pngdec.c | 11 +----------
> >  1 file changed, 1 insertion(+), 10 deletions(-)
> >
> > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > index 2512799..2ea3e8b 100644
> > --- a/libavcodec/pngdec.c
> > +++ b/libavcodec/pngdec.c
> > @@ -1224,17 +1224,10 @@ static int update_thread_context(AVCodecContext
> *dst, const AVCodecContext *src)
> >      if (dst == src)
> >          return 0;
> >
> > -    pdst->frame_id = psrc->frame_id;
> > -
> >      ff_thread_release_buffer(dst, &pdst->picture);
> >      if (psrc->picture.f->data[0] &&
> >          (ret = ff_thread_ref_frame(&pdst->picture, &psrc->picture)) < 0)
> >          return ret;
> > -    if (CONFIG_APNG_DECODER && dst->codec_id == AV_CODEC_ID_APNG) {
> > -        ff_thread_release_buffer(dst, &pdst->last_picture);
> > -        if (psrc->last_picture.f->data[0])
> > -            return ff_thread_ref_frame(&pdst->last_picture,
> &psrc->last_picture);
> > -    }
> >
> >      return 0;
> >  }
> > @@ -1294,9 +1287,7 @@ AVCodec ff_apng_decoder = {
> >      .init           = png_dec_init,
> >      .close          = png_dec_end,
> >      .decode         = decode_frame_apng,
> > -    .init_thread_copy = ONLY_IF_THREADS_ENABLED(png_dec_init),
> > -    .update_thread_context =
> ONLY_IF_THREADS_ENABLED(update_thread_context),
> > -    .capabilities   = CODEC_CAP_DR1 | CODEC_CAP_FRAME_THREADS /*|
> CODEC_CAP_DRAW_HORIZ_BAND*/,
> > +    .capabilities   = CODEC_CAP_DR1 /*| CODEC_CAP_DRAW_HORIZ_BAND*/,
> >  };
> >  #endif
> >
> > --
> > 2.4.0
> >
>
> Hmm, have you checked it is really broken now?
>

It wasn't broken originally, but breaks when I fix the frame disposing.
More specifically, since each frame depends on the metadata of the previous
frame as well as the frame itself, having it threaded would give
inconsistent last-frame metadata, unless we wait for the frame before
reading the next frame's metadata.
But if we wait for the frame before reading the next frame, we've
effectively made it only execute a single thread at a time, which is why
I've removed threading support altogether.

I suppose another possible solution is to only have decompression be
multithreaded while everything else is single threaded.