On Fri, 8 Dec 2017 06:52:20 +0100 Michael Niedermayer <mich...@niedermayer.cc> wrote:
> 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 <mich...@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 > > >>>>> <mich...@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 > > >>>>>>> <mich...@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 > > >>>>>>>>> <mich...@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 > > >>>>>>>>>>> <mich...@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) Also definitely wrong API usage. > and its our only bug set to that priority. Since when does the bug tracker mean anything? Bug reports that are wrt. my project are literally ignored. > 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 > > [...] _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
