GNOME Bugzilla – Bug 772143
qtmux: Don't calculate PTS offset and DTS with GST_CLOCK_TIME_NONE
Last modified: 2016-09-30 07:06:04 UTC
See commit message
Created attachment 336473 [details] [review] qtmux: Don't calculate PTS offset and DTS with GST_CLOCK_TIME_NONE The current code assumes that there is always a PTS, which is not necessarily true. Handle the cases where either PTS or DTS are not available, and error out properly if neither are, instead of calculating completely wrong values.
Review of attachment 336473 [details] [review]: The collectpad clip method use to ensure we have both set as soon as one of DTS or PTS is present. It remains that it would be cleaner to ensure that first and keep the if/else stuff simple.
My patch merely keeps behaviour and stops qtmux from doing stupid calculations with GST_CLOCK_TIME_NONE. I'm sure there is more broken in here, but that's for a later time :) I just saw this bug here now because of another bug elsewhere where wrong timestamps were calculated.
Question is mostly, should we get this in for now? Should any of the cases be an error instead (e.g. should no PTS be an error)? What are the requirements here? We always need PTS, but if there ever was a DTS we also *always* need a DTS? I could implement that too, easy enough ;) The collectpads clip function currently does not ensure that whenever there is a DTS, that both are there. It has no state other than the segment currently.
I believe it used to, but I might be wrong. My idea was to make sure we ensure both as soon as possible, and then the rest of the code can just assume it exist. Otherwise it becomes hairy. This is related to blocker bug https://bugzilla.gnome.org/show_bug.cgi?id=762207
We could also just merge the patches aimed at 1.8 in both, and keep going without lives.
So you propose to add a patch to that collectpads clip function to assume that DTS==PTS if there is no PTS? That would also solve this bug here, except for the case where there is neither DTS nor PTS :)
My opinion is that feeding data without PTS into qtmux is an error and we should not make up stuff (ie. potentially produce broken files) if there is no PTS on the input. I realise that's inconvenient for some, but it should be fixed elsewhere.
Yes I agree with that. So I could update my patch to just error out if there is no PTS, and that will also solve it
++
Attachment 336473 [details] pushed as a993883 - qtmux: Don't calculate PTS offset and DTS with GST_CLOCK_TIME_NONE