[FFmpeg-devel] [PATCH 4/6] Revert "avcodec/pngdec: fix possible race condition with APNG decoding"

Paul B Mahol onemda at gmail.com
Wed Feb 17 12:45:02 EET 2021


On Wed, Feb 17, 2021 at 9:14 AM Anton Khirnov <anton at khirnov.net> wrote:

> Quoting Paul B Mahol (2021-02-16 21:57:47)
> > Do you have actual proof for such claims?
>
> The burden of proof is on you here - you are supposed to prove that your
> commit fixes something. And when I asked you about details, you couldn't
> even tell me what the bug was, much less why would switching the order
> of allocations be the correct fix for it.
>

It is not about order of allocations.


>
> >
> > There is no point in reverting commit if that commit fixed some behavior.
>
> It didn't fix anything though. Before your commit, frame 69 of the
> sample in #9017 is almost black with 1 thread and looks okay with 2
> threads. After your commit, it is almost black in both cases. So your
> commit made it consistently wrong. This shows up in my tests and is also
>


You are mistaken. It fixed race. Prove it does not.
Another patch is supposed to fix output for both single and multi threads.



> supported by the last comment in https://trac.ffmpeg.org/ticket/9017
> Feel free to test it yourself.
>
> Next not reviewed patch fixed it completely.
Thanks for actually confirming it makes consistent result with different
threads and as such commit should not
be reverted.

If you have no proof that your changes too apng code improves anything
besides line counts than
I nack whole patch set.

I have not tested your set at all. But plan to if you provide file you used
to actually test this and that file is different from already provided file
on trac.

-- 
> Anton Khirnov
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".


More information about the ffmpeg-devel mailing list