On Tue, 4 Oct 2016 16:35:02 +0200 Hendrik Leppkes <h.lepp...@gmail.com> wrote:
> On Tue, Oct 4, 2016 at 4:32 PM, wm4 <nfx...@googlemail.com> wrote: > > On Tue, 4 Oct 2016 14:15:03 +0200 > > Michael Niedermayer <mich...@niedermayer.cc> wrote: > > > >> On Tue, Oct 04, 2016 at 01:52:02PM +0200, Hendrik Leppkes wrote: > >> > On Tue, Oct 4, 2016 at 1:44 PM, Hendrik Leppkes <h.lepp...@gmail.com> > >> > wrote: > >> > > On Tue, Oct 4, 2016 at 1:23 PM, Michael Niedermayer > >> > > <mich...@niedermayer.cc> wrote: > >> > >> On Tue, Oct 04, 2016 at 08:41:42AM +0200, Hendrik Leppkes wrote: > >> > >>> On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer > >> > >>> <mich...@niedermayer.cc> wrote: > >> > >>> > On Sat, Oct 01, 2016 at 04:15:45PM +0200, Hendrik Leppkes wrote: > >> > >>> >> Decoders have previously not used AVFrame.pts, and with the > >> > >>> >> upcoming > >> > >>> >> deprecation of pkt_pts (in favor of pts), this would lead to an > >> > >>> >> errorneous > >> > >>> >> interpration of timestamps. > >> > >>> > > >> > >>> > I probably misunderstand the commit message but > >> > >>> > If code is changed in a user application that cannot really lift > >> > >>> > some blockage from changing a lib. > >> > >>> > a lib can only change in an incompaible way with (deprecation and) > >> > >>> > major version bump. > >> > >>> > > >> > >>> > >> > >>> The lib never did what this code suggests it did, not that I remember > >> > >>> (so at least not for a long long time). > >> > >> > >> > >> release/2.0 with > >> > >> > >> > >> diff --git a/libavcodec/utils.c b/libavcodec/utils.c > >> > >> index 29d5492..57c8d50 100644 > >> > >> --- a/libavcodec/utils.c > >> > >> +++ b/libavcodec/utils.c > >> > >> @@ -2008,7 +2008,7 @@ int attribute_align_arg > >> > >> avcodec_decode_video2(AVCodecContext *avctx, AVFrame *pi > >> > >> avci->to_free.extended_data = avci->to_free.data; > >> > >> memset(picture->buf, 0, sizeof(picture->buf)); > >> > >> } > >> > >> - > >> > >> +av_assert0(picture->pts == 0 || picture->pts == AV_NOPTS_VALUE); > >> > >> avctx->frame_number++; > >> > >> av_frame_set_best_effort_timestamp(picture, > >> > >> > >> > >> guess_correct_pts(avctx, > >> > >> > >> > >> causes many tests to fail, indicating that AVFrame.pts was set for > >> > >> several video decoders, the ffmpeg code is audio decoder specific > >> > >> and i failed to find a case where it was triggered, i tried IIRC 3 > >> > >> or so checkouts from the past > >> > >> > >> > >> so AVFrame.pts was maybe never set for decoding audio but it was set > >> > >> for video > >> > > > >> > > Can you extend the test to add "|| picture->pts == picture->pkt_pts"? > >> > > Because thats what it would be set to after the merge. A quick check > >> > > in the 2.0 code base looks like some decoders did set that, but to the > >> > > exact same value as pkt_pts (which is what the merge is proposing > >> > > right now as well) > >> > > > >> > > >> > And I found this (after 2.0): > >> > http://git.videolan.org/?p=ffmpeg.git;a=commitdiff;h=a1c5cc429d99216406170eac7e8352860076d3e8 > >> > > >> > Which apparently set pts for mpeg4 to a number parsed from the > >> > bitstream, entirely uncorrelated to container or audio timestamps, so > >> > using them would have been rather impractical for any real use-cases. > >> > >> They can be usfull, some random examples: > >> > >> playing MPEG4-ES with timing stored from the bitstream (assuming there > >> is no demuxer lib used that is capable to extract them) > >> > >> forensics, raw video stream could have its timing > >> recovered, a video with manipulated container timestamps could be > >> detected. > >> > >> error correction, if the container level timestamps are lost or > >> corrupted the stream level ones can be used to recreate them > >> > >> There may be more, these are just some of the top of my head, > >> not your mainstream multimedia player stuff maybe but they arent > >> useless > >> > >> [...] > >> > > > > They don't belong into the AVFrame.pts field, though. > > And they don't go in there anymore right now, so thats that. > > The real question is, what do we do about this merge now? > Can we set AVFrame.pts to the same value as AVFrame.pkt_pts safely, > considering it was unused in the current ABI/API, or would that be > considered an API break and we better delay this change until the next > major? IMO applications which did this were pretty broken anyway, and we should be able to get away with a simple version bump. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
