[FFmpeg-devel] [PATCH 8/8] avcodec/decode: copy the output parameters from the last bsf in the chain back to the AVCodecContext

[James Almer]

On 7/28/2018 6:28 PM, Michael Niedermayer wrote:
> On Sat, Jul 28, 2018 at 01:25:36PM -0300, James Almer wrote:
>> On 7/28/2018 4:09 AM, Michael Niedermayer wrote:
>>> On Fri, Jul 27, 2018 at 11:11:47PM -0300, James Almer wrote:
>>>> On 7/27/2018 10:58 PM, Michael Niedermayer wrote:
>>>>> On Fri, Jul 27, 2018 at 11:57:49AM -0300, James Almer wrote:
>>>>>> Certain AVCodecParameters, like the contents of the extradata, may be changed
>>>>>> by the init() function of any of the bitstream filters in the chain.
>>>>>>
>>>>>> Signed-off-by: James Almer <jamrial at gmail.com>
>>>>>> ---
>>>>>> Now it's not going to be called after the codec has been opened.
>>>>>>
>>>>>>  libavcodec/decode.c | 4 ++++
>>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> This breaks:
>>>>> ffmpeg -i https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png -bitexact -pix_fmt rgba -f framecrc -
>>>>
>>>> Is any other decoder failing the same way? Because the apng decoder
>>>> threading code may be faulty otherwise. Plenty of avctx fields are
>>>> changed after ff_thread_init() is called within avcodec_open2(). There
>>>> should not be a race at this point.
>>>
>>> I found a failure with mpeg4 (with bframes) decoding + pcm_alaw from mkv but it
>>> does not want to reproduce. The slightest change i do makes this not happen
>>> even just duplicating a command line parameter (which should have no effect)
>>> simply adding the -threads parameter to it independant of value makes it go away
>>> too
>>>
>>>
>>> in the png case
>>> this hits teh issue:
>>> -threads 2 -i https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png  -f framecrc -
>>>
>>> this does not:
>>> -threads 1 -i https://upload.wikimedia.org/wikipedia/commons/1/14/Animated_PNG_example_bouncing_beach_ball.png  -f framecrc -
>>>
>>> also odly the bitexact flag made a differnce in how it fails outside valgrind
>>> last i tried. (doesnt make a difference in valgrind it seems)
>>
>> A solution may be moving the ff_decode_bsfs_init call in patch 7/8 right
>> above the call to ff_thread_init (See attachment), hopefully preventing
>> this race once this patch is applied afterwards, but it will result in
>> the bsfs initialized before the decoder, and some of the avctx fields
>> that are changed later in avcodec_open2 like channels and bit_rate not
>> being reflected during said bsfs initialization.
>> I can't say if the former is correct or ideal, but for now the latter
>> would not be an issue. I don't know what may happen if we were to
>> autoinsert a filter that does care about such fields in the future, though.
>>
>> If the above change doesn't solve it, or is not ideal, then this patch
>> 8/8 can be dropped or postponed, and the rest of the set pushed without it.
> 
> with this patch, the pachset seems not to trigger these errors anymore
> 
> thanks

Set pushed. Thanks.