[FFmpeg-devel] [PATCH] ffmpeg: remove unused and errorneous AVFrame timestamp check

[Michael Niedermayer]

On Tue, Oct 04, 2016 at 10:30:24AM +0200, Hendrik Leppkes wrote:
> On Tue, Oct 4, 2016 at 8:41 AM, Hendrik Leppkes <h.leppkes at gmail.com> wrote:
> > On Tue, Oct 4, 2016 at 4:05 AM, Michael Niedermayer
> > <michael at 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).
> >
> 
> Of course that could still mean that some other apps "copied" the
> ffmpeg code and try to read this field - but is this a scenario we can
> really control?
> 
> The pts field in AVFrame is currently unused for decoding, nothing
> sets it (except cuvid and openh264 or so, but those set it the same
> way it would be set in the future, so no changes there), ffmpeg.c
> trying to read it is a remnant from a long time ago (quick blame pins
> it at 2012 when decoding was changed to decode_audio4).
> I couldn't actually confirm if at that time (audio) decoders did even
> set AVFrame.pts, considering pkt_pts already existed then.
> 
> So is starting to set a field that was previously (at least through 2
> or so major bumps) unused a API break?
> Its always possible some app still tries to read it, but because its
> never set it didn't cause any problems so far.
> 
> The alternative is of course to keep using pkt_pts, and keep pts
> unused for decoding, but I'm not entirely convinced there is a break
> here.

not stating any oppinion in this paragraph but
If use of AVFrame.pts is considered a bug in the current API then past
releases with that API need to be fixed. If they are not fixed
testing API/ABI for 3.2 will blow up (i havnt tried it yet for 3.2
but i did for past releases previously and mixing libs between releases
and ffmpeg is required to not blow up, it protects against API/ABI
breaks somewhat)
Also release notes for 3.2 would be needed as current 3.1 would not
mix well with a release with same soname and differently used
AVFrame.pts. That is unless i miss something

Somewhat off topic and my personal oppinion
I think independant of field names and API, there are 3 or 4 types of
timestamps, it would be good if user applications have some easy
way of accessing all of them for a frame from a decoder.
the 4 types are,
* input AVPacket.pts based
* input AVPacket.dts based
* what is stored in the codec bitstream if any
* some easy to use one that simple apps can just use and not need to
  worry about anything, aka one that is "correct" in almost all cases


And of course i want the next release to work for distros and not
see complaints about it causing pain due to API/ABI stuff, breaking
packages and so on.

And i want a clean, easy to use, maintainable and yet powerfull API

About the original patch in this thread itself i have no real oppinon

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

Democracy is the form of government in which you can choose your dictator
-------------- 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/20161004/3da3866e/attachment.sig>