[FFmpeg-devel] [PATCH] libavcodec/pngdec: support 'previous' dispose operation for APNG.

[Benoit Fouet]

Hi,

On December 1, 2014 11:34:44 PM GMT+01:00, Michael Niedermayer <michaelni at gmx.at> wrote:
>On Mon, Dec 01, 2014 at 11:41:41AM +0100, Benoit Fouet wrote:
>> ---
>> Tested against all the materials I have at hand.
>> There is an artifact showing for
>https://raw.githubusercontent.com/maxcom/lorsource/master/src/test/resources/images/i_want_to_be_a_hero__apng_animated__by_tamalesyatole-d5ht8eu.png
>> which I don't really understand, as it seems the individual frames
>are correct
>> for our decoder, but the disposal that's done for other decoders
>(tested
>> firefox and chrome) is not the same for the end of the cape.
>> ---
>>  libavcodec/pngdec.c | 93
>++++++++++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 71 insertions(+), 22 deletions(-)
>> 
>> diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
>> index 9e52d0b..2ca3dee 100644
>> --- a/libavcodec/pngdec.c
>> +++ b/libavcodec/pngdec.c
>> @@ -23,6 +23,7 @@
>>  
>>  #include "libavutil/bprint.h"
>>  #include "libavutil/imgutils.h"
>> +#include "libavutil/thread.h"
>>  #include "avcodec.h"
>>  #include "bytestream.h"
>>  #include "internal.h"
>> @@ -38,9 +39,16 @@ typedef struct PNGDecContext {
>>      AVCodecContext *avctx;
>>  
>>      GetByteContext gb;
>> +    ThreadFrame previous_picture;
>>      ThreadFrame last_picture;
>>      ThreadFrame picture;
>>  
>> +#if CONFIG_APNG_DECODER
>> +    AVMutex mutex;
>> +    int frame_id;
>> +    int *pframe_id;
>> +#endif
>
>why do you need a mutex ?
>

Actually, the only thing I need is the frame index. The best place for that would be in the demuxer, but I didn't find a place where this information is accessible. Did I miss something (I hope so)? Do you think I should be using side data for this?
To answer the question though, the access is done is all the decoder threads, so I did not want the reset to happen between the reading and the writing of the ++.
Thinking more about this, I think it's wrong anyway. I really need the demuxer to handle this, it would be simpler and more correct...

-- 
Ben