Hi,
On Wed, Jun 3, 2015 at 1:20 AM, Donny Yang <w...@kota.moe> wrote:
> On 3 June 2015 at 04:15, Ronald S. Bultje <rsbul...@gmail.com> wrote:
> > On Tue, Jun 2, 2015 at 1:42 PM, Donny Yang <w...@kota.moe> wrote:
> > > On 3 June 2015 at 03:31, Paul B Mahol <one...@gmail.com> wrote:
> > > > Dana 2. 6. 2015. 17:49 osoba "Donny Yang" <w...@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 <w...@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.
> >
> >
> > What is this metadata? I'm fairly confused over this.
> >
>
> Metadata as referring to the data contained in the fcTL chunks, stored
> inside PNGDecContext in ffmpeg.
> More specifically, these properties:
> int last_w, last_h;
> int last_x_offset, last_y_offset;
> uint8_t last_dispose_op;
>
> For example, when I tested with
> https://people.mozilla.org/~dolske/apng/demo-2-over+previous.png
> last_dispose_op would always be APNG_DISPOSE_OP_NONE rather than
> APNG_DISPOSE_OP_PREVIOUS
> for the second frame. the last_* properties were probably also wrong, but I
> didn't check those.
> Disabling threading fixed it.
So sync these struct members in the update_thread_context callback (check
mpeg/h26x/vpx codecs for examples)? Disabling threading seems like a bit
much.
Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
