On Sat, Dec 09, 2017 at 09:46:53PM +0100, Marton Balint wrote: > > > On Mon, 27 Nov 2017, Michael Niedermayer wrote: > > >On Sun, Nov 26, 2017 at 12:09:35PM -0300, James Almer wrote: > >>On 11/21/2017 6:48 PM, Marton Balint wrote: > >>> > >>> > >>>On Thu, 9 Nov 2017, James Cowgill wrote: > >>> > >>>>Hi, > >>>> > >>>>On 09/11/17 14:02, Hendrik Leppkes wrote: > >>>>>On Thu, Nov 9, 2017 at 1:21 PM, James Cowgill <jcowg...@debian.org> > >>>>>wrote: > >>>>>>In commit 061a0c14bb57 ("decode: restructure the core decoding > >>>>>>code"), the > >>>>>>deprecated avcodec_decode_* APIs were reworked so that they called > >>>>>>into the > >>>>>>new avcodec_send_packet / avcodec_receive_frame API. This had the > >>>>>>side effect > >>>>>>of prohibiting sending new packets containing data after a drain > >>>>>>packet, but in previous versions of FFmpeg this "worked" and some > >>>>>>applications relied on it. > >>>>>> > >>>>>>To restore some compatibility, reset the codec if we receive a new > >>>>>>non-drain > >>>>>>packet using the old API after draining has completed. While this does > >>>>>>not give the same behaviour as the old API did, in the majority of > >>>>>>cases > >>>>>>it works and it does not require changes to any other part of the > >>>>>>decoding > >>>>>>code. > >>>>>> > >>>>>>Fixes ticket #6775 > >>>>>>Signed-off-by: James Cowgill <jcowg...@debian.org> > >>>>>>--- > >>>>>> libavcodec/decode.c | 5 +++++ > >>>>>> 1 file changed, 5 insertions(+) > >>>>>> > >>>>>>diff --git a/libavcodec/decode.c b/libavcodec/decode.c > >>>>>>index 86fe5aef52..2f1932fa85 100644 > >>>>>>--- a/libavcodec/decode.c > >>>>>>+++ b/libavcodec/decode.c > >>>>>>@@ -726,6 +726,11 @@ static int compat_decode(AVCodecContext *avctx, > >>>>>>AVFrame *frame, > >>>>>> > >>>>>> av_assert0(avci->compat_decode_consumed == 0); > >>>>>> > >>>>>>+ if (avci->draining_done && pkt && pkt->size != 0) { > >>>>>>+ av_log(avctx, AV_LOG_WARNING, "Got unexpected packet after > >>>>>>EOF\n"); > >>>>>>+ avcodec_flush_buffers(avctx); > >>>>>>+ } > >>>>>>+ > >>>>> > >>>>>I don't think this is a good idea. Draining and not flushing > >>>>>afterwards is a bug in the calling code, and even before recent > >>>>>changes it would result in inconsistent behavior and even crashes > >>>>>(with select decoders). > >>>> > >>>>I am fully aware that this will only trigger if the calling code is > >>>>buggy. I am trying to avoid silent breakage of those applications doing > >>>>this when upgrading to ffmpeg 3.4. > >>>> > >>>>I was looking at the documentation of avcodec_decode_* recently because > >>>>of this and I had some trouble deciding if using the API this way was > >>>>incorrect. I expect the downstreams affected thought that what they were > >>>>doing was fine and then got angry when ffmpeg suddenly "broke" their > >>>>code. This patch at least allows some sort of "transitional period" > >>>>until downstreams update. > >>> > >>>I think the intent was to flush the codec by passing the NULL packets to > >>>it, so it makes a lot of sense to actually do that. Especially since by > >>>implicitly doing a flush, we can avoid the undefined behaviour/crashes > >>>on the codec side. > >>> > >>>Also this is only compatibility code, which probably will be removed at > >>>the next bump, I see no harm in making it as compatible as realistically > >>>possible. > >> > >>The old decode API is not scheduled for removal right now probably > >>because 99% of decoders need to be ported. > >>This compat code was written so the old API becomes a wrapper for the > >>new rather than the other way around, as it was up to 3.3. Supposedly a > >>good portion of the versatility of the new API would be handicapped > >>otherwise. > >> > > > >>Personally, I think this should be left as is. It is a good incentive > >>for downstream to migrate to the new API, as they technically were > >>misusing the old API to begin with. > > > >providing compatibility support for an old API that does not actually > >work with how applications used the old API is something a tad bit > >bizzare > > > >We want to minimize the work everyone has to do. > >The more time people have, the more they can spend on improving free > >software. > > > >If the old API is going to be removed, any work people have to do > >to hunt and track implementation changes in our old API the more > >they have wasted time. > >If you want people to spend their time on the new API, then you > >should not introduce issues in the old API that they need to > >workaround > >They that way just lost time (debug, fix, test) they could have spend > >on the new API or on anything else > > > > Applied and backported.
Thanks alot ! [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB It is what and why we do it that matters, not just one of them.
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel