On Tue, Nov 21, 2017 at 10:48:19PM +0100, 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.
i agree and i would appreciate if this gets resolved one way or another so i can make the release Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The educated differ from the uneducated as much as the living from the dead. -- Aristotle
signature.asc
Description: Digital signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel