[FFmpeg-devel] FFmpeg 3.4.1

Michael Niedermayer michael at niedermayer.cc
Fri Dec 8 07:52:20 EET 2017


On Fri, Dec 08, 2017 at 01:09:50AM -0300, James Almer wrote:
> On 12/8/2017 12:26 AM, wm4 wrote:
> > On Thu, 7 Dec 2017 23:23:51 +0100
> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> > 
> >> On Tue, Nov 21, 2017 at 07:58:18PM +0100, Michael Niedermayer wrote:
> >>> On Sat, Nov 18, 2017 at 09:11:17PM +0100, Michael Niedermayer wrote:  
> >>>> On Sat, Nov 18, 2017 at 09:50:33AM +0100, Hendrik Leppkes wrote:  
> >>>>> On Sat, Nov 18, 2017 at 3:05 AM, Michael Niedermayer
> >>>>> <michael at niedermayer.cc> wrote:  
> >>>>>> On Fri, Nov 17, 2017 at 09:55:47AM +0100, Hendrik Leppkes wrote:  
> >>>>>>> On Fri, Nov 17, 2017 at 5:00 AM, Michael Niedermayer
> >>>>>>> <michael at niedermayer.cc> wrote:  
> >>>>>>>> On Thu, Nov 16, 2017 at 01:51:34PM +0100, Carl Eugen Hoyos wrote:  
> >>>>>>>>> 2017-11-16 13:44 GMT+01:00 Michael Niedermayer <michael at niedermayer.cc>:  
> >>>>>>>>>> On Thu, Nov 16, 2017 at 01:04:27AM +0100, Carl Eugen Hoyos wrote:  
> >>>>>>>>>>> 2017-11-15 13:34 GMT+01:00 Michael Niedermayer <michael at niedermayer.cc>:  
> >>>>>>>>>>>> Hi all
> >>>>>>>>>>>>
> >>>>>>>>>>>> I intend to make 3.4.1 very soon  
> >>>>>>>>>>>
> >>>>>>>>>>> Shouldn't we first decide on how to proceed with #6775?  
> >>>>>>>>>>
> >>>>>>>>>> This would be ideal.
> >>>>>>>>>>
> >>>>>>>>>> IIUC this is a regression from bddb2343b6e594e312dadb5d21b408702929ae04  
> >>>>>>>>>
> >>>>>>>>> This was confirmed by more than one developer, yes.
> >>>>>>>>>  
> >>>>>>>>>> I see a patch that is said to improve 6775, can that be applied and
> >>>>>>>>>> would that resolve this ?  
> >>>>>>>>>
> >>>>>>>>> Iiuc, it would not completely resolve the issue, see:
> >>>>>>>>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=881536
> >>>>>>>>>  
> >>>>>>>>>> If so why was it not applied yet ?  
> >>>>>>>>>
> >>>>>>>>> The patch did not get support here, see:
> >>>>>>>>> [FFmpeg-devel] [PATCH] lavc: reset codec on receiving packet after EOF
> >>>>>>>>> in compat_decode  
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Is someone working on fixing this better than with the available patch ?
> >>>>>>>>  
> >>>>>>>
> >>>>>>> I don't even agree this needs fixing. Those projects use the API wrong.  
> >>>>>>
> >>>>>> Had we documented the correct/wrong use precissely in the past when
> >>>>>> these wrong uses where written ?
> >>>>>>
> >>>>>> Because if it was documented then few should have made the mistake.
> >>>>>> But it seems this affects multiple projects, so i wonder if our API
> >>>>>> really excluded the use
> >>>>>>  
> >>>>>
> >>>>> Apparently not well enough, but I also don't even understand why you
> >>>>> would *want* to drain in the middle of decoding.
> >>>>>
> >>>>> The only mention of sending NULL/0 packets (in 3.0 docs, before the
> >>>>> new API was introduced) do include the "at the end", however.
> >>>>>
> >>>>> From CODEC_CAP_DELAY:
> >>>>>  * Decoders:
> >>>>>  * The decoder has a non-zero delay and needs to be fed with avpkt->data=NULL,
> >>>>>  * avpkt->size=0 at the end to get the delayed data until the decoder no longer
> >>>>>  * returns frames.
> >>>>>
> >>>>> From avcodec_decode_video2
> >>>>> * @note Codecs which have the AV_CODEC_CAP_DELAY capability set have a delay
> >>>>> * between input and output, these need to be fed with avpkt->data=NULL,
> >>>>> * avpkt->size=0 at the end to return the remaining frames.
> >>>>>
> >>>>> There is a few more mentions of the same concept, but as far as I can
> >>>>> see all include the key words "at the end".
> >>>>>   
> >>>>   
> >>>>> For the suggested patch, draining and flushing in the middle of a
> >>>>> bitstream is still going to result in problems, though, since it
> >>>>> removes all reference frames, for example.
> >>>>> The original behavior cannot really be stored, which was to just keep
> >>>>> feeding frames into the decoder after a drain without a flush.
> >>>>> However, some decoders actually crashed when you did this, so this was
> >>>>> a rather unsafe action to begin with (not an issue any longer, since
> >>>>> this pattern is now blocked).  
> >>>>
> >>>> Did the previous code really work if the frame after a flush was not a
> >>>> new keyframe or there was some use of previous references ?  
> >>>
> >>> ping
> >>>
> >>> so what is the status of this?
> >>>
> >>> Ticket 6775 is still open, neither a workaround was applied nor was
> >>> it closed as invalid. Only one workaround was proposed which was
> >>> claimed to be worse than the code before.
> >>> It seems the discussion died down.
> >>> If theres no activity on this in the next days then i intend to make
> >>> the release with whats in release/3.4 at the time. I dont think
> >>> blind waiting will do any good, id rather release early and often ...
> >>>
> >>> Also if someone wants to write some release notes about this issue,
> >>> that is IMO a good idea ...  
> >>
> >> So this code is completely unmaintained ?
> >> Noone cares about pushing a workaround ?
> >> Noone cares about closing the ticket as wont fix ?
> >> Noone cares about explaining why neither should be done ?
> >>
> >> I intend to make the release tomorrow or as soon as i have time, we
> >> have waited too long already. I can of course wait more if people want
> >> but then please have a plan on resolving the issue that the release is
> >> delayed for
> > 
> > I didn't read all of this, but it's probably due to the API user
> > passing as null packet, which indeed signals EOF. What happens if you
> > send more packets after sending EOF has always been undefined behavior,
> > and some audio decoders could crash if you did that. (It's fine if
> > you flush the decoder, of course.) I think I brought it up in the past
> > (and nobody cared), so I added explicit workarounds to my own API usage
> > code to avoid crashes. Why are you making so much noise around it now?
> 
> When the old decode API was turned into a wrapper for the new, some
> applications using said API this way started to experience
> issues/crashes that did not happen before. So basically, a change of
> behavior which, as you put it, was apparently undefined.
> 
> Some argue it should not crash if it didn't before, others argue that it
> was never meant to be used this way. What Michael wants to know in order
> to release 3.4.1 is, what should be done? Should it be addressed, or
> left as is? If the former, do we apply the patch someone proposed in
> another thread, or something else? If the latter, then the ticket should
> be closed and the discussion ends there.

yes
also to add to this, 6775 is set to the highest priority (critical)
and its our only bug set to that priority.
The priority likely makes sense from a distribution packagers
point of view. And while it might not make sense from our point of
view to set the priority to that in a vacuum. We should care a bit
about downstream and what causes significant pain to them.

that said delaying releases for issues that noone works on is bad

Thanks

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

Many things microsoft did are stupid, but not doing something just because
microsoft did it is even more stupid. If everything ms did were stupid they
would be bankrupt already.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20171208/c0c568d4/attachment.sig>


More information about the ffmpeg-devel mailing list