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

[Michael Niedermayer]

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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180728/ccb435c9/attachment.sig>