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