Re: [FFmpeg-devel] [PATCH v4 2/4] mpegts: Stash original PTS for SCTE-35 sections for processing later

From: Kieran Kunhya <kierank@obe.tv>
To: Devin Heitmueller <devin.heitmueller@ltnglobal.com>
Cc: Kieran Kunhya <kierank@obe.tv>,
	Devin Heitmueller <dheitmueller@ltnglobal.com>,
	FFmpeg development discussions and patches
	<ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v4 2/4] mpegts: Stash original PTS for SCTE-35 sections for processing later
Date: Thu, 10 Aug 2023 09:08:59 -0400
Message-ID: <CAK+ULv6o-ppsvg+NCjQ3SCRps06NcOTC7YHjv0wumtKN886PNw@mail.gmail.com> (raw)
In-Reply-To: <CAHGibzELyFFYcs7hvWq_e5JtRgtG_NYpq9C++7jc6zj3Jc8ryg@mail.gmail.com>

On Thu, 10 Aug 2023, 08:59 Devin Heitmueller, <
devin.heitmueller@ltnglobal.com> wrote:

> On Thu, Aug 10, 2023 at 8:48 AM Kieran Kunhya <kierank@obe.tv> wrote:
> >
> >
> >
> > On Thu, 10 Aug 2023 at 08:41, Kieran Kunhya <kierank@obe.tv> wrote:
> >>
> >> On Thu, 10 Aug 2023 at 08:20, Devin Heitmueller <
> devin.heitmueller@ltnglobal.com> wrote:
> >>>
> >>> On Thu, Aug 10, 2023 at 8:13 AM Kieran Kunhya <kierank@obe.tv> wrote:
> >>> > The (closest?) video PTS is even worse than the last PCR because the
> VBV means the closest PTS can be quite far from the interpolated PCR.
> >>>
> >>> It's arguments like that which prompted me to explicitly exclude such
> >>> a patch from the series.  It's a discussion to be had, but not
> >>> relevant for this patch series (which makes no effort to change the
> >>> logic for how the timestamp is determined).
> >>>
> >>> Wait until such a patch is submitted, and then we can debate at length
> >>> the ambiguity in the specification and what the best approach is.
> >>
> >>
> >> There is zero ambiguity in the specification.
> >
> >
> > Like any other form of SI in MPEG-TS the timestamp (although there is
> actually no such thing) is "now", which by definition is the interpolated
> PCR.
> > Taking a video frame PTS is incorrect.
> >
> > What's the point of submitting patches like this exposing things in the
> public API that you know to be wrong?
>
> Again, this patch series makes no attempt to address the problem you
> are complaining about.  It brings the situation from "completely
> doesn't work" to "works fine the majority of the time except for the
> splice immediate case where the timestamp may not be as accurate as it
> could be".  And let's be fair, splice immediate is both an uncommon
> use case in the industry and nobody doing a splice immediate expects
> it to be frame accurate (as it's typically initiated by a human during
> live programming).
>
> I'm happy to have this discussion, but it doesn't have any bearing on
> whether this patch series should be accepted.  Let's not throw out the
> baby with the bathwater.
>

You're exposing this incorrect information as public API, two wrongs don't
make a right.

I also told the author of the previous code that it was wrong but the patch
was forced through on the guise that "professionals won't respect ffmpeg if
scte-35 isn't demuxed".

The fact something isn't used often, doesn't mean it should be implemented
badly. You could say that interlaced isn't used much as a total of all the
video in the world so we should just not decode it correctly.

By all means keep your hacks in your forks.

Kieran

>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".