Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Devin Heitmueller <devin.heitmueller@ltnglobal.com>
To: Kieran Kunhya <kierank@obe.tv>
Cc: 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 11:12:21 -0400
Message-ID: <CAHGibzHQPXH-Eg6_RdYtGZpQnWg33hURAYVyXH+7rPY4k6_C2g@mail.gmail.com> (raw)
In-Reply-To: <CAK+ULv6o-ppsvg+NCjQ3SCRps06NcOTC7YHjv0wumtKN886PNw@mail.gmail.com>

Hi Kieran,

On Thu, Aug 10, 2023 at 9:09 AM Kieran Kunhya <kierank@obe.tv> wrote:
> You're exposing this incorrect information as public API, two wrongs don't make a right.

The information is already exposed via the public API, in the form of
pkt->pts.  The fact that I've exposed that same value as side data as
well doesn't really change that situation.

> 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".

So you're annoyed that seven years ago somebody else submitted some
patch and it got merged over your objection?  And that in that time
nobody (including you) has cared enough to take it upon themselves to
improve the accuracy of the timestamp?

> 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.

I've made no attempt to argue that the accuracy of the timestamp
couldn't be improved.  I would argue that improving it shouldn't be
any sort of prerequisite to merging this patch series though.
Presumably it hasn't happened already because 1) properly implementing
PCR interpolation within the ffmpeg mpegts demux implementation is
difficult, 2) nobody has cared enough to do it, and/or 3) the typical
PCR interval is 40ms so people are happy with it being "close enough"
in the splice immediate case.

> By all means keep your hacks in your forks.

Where is the hack?  My patches are an incremental improvement over
what is already there.  The notion that it makes no attempt to  solve
some orthogonal problem that I'm not really worried about isn't
grounds for rejecting it.

I would be happy to see other developers weigh in, or if Kieran's is
the final word then I would like to see this reviewed by the TC.  I'm
making a genuine effort to improve the support in ffmpeg and it seems
like an artificial barrier is being raised against those efforts.

Devin


Devin

--
Devin Heitmueller, Senior Software Engineer
LTN Global Communications
o: +1 (301) 363-1001
w: https://ltnglobal.com  e: devin.heitmueller@ltnglobal.com
_______________________________________________
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".

  reply	other threads:[~2023-08-10 15:12 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-31 13:38 [FFmpeg-devel] [PATCH v4 0/4] Add passthrough support for SCTE-35 Devin Heitmueller
2023-07-31 13:38 ` [FFmpeg-devel] [PATCH v4 1/4] avcodec: Add new side data type to contain original PTS value Devin Heitmueller
2023-07-31 13:38 ` [FFmpeg-devel] [PATCH v4 2/4] mpegts: Stash original PTS for SCTE-35 sections for processing later Devin Heitmueller
2023-08-09 13:54   ` Kieran Kunhya
2023-08-09 16:36     ` Devin Heitmueller
2023-08-10 12:12       ` Kieran Kunhya
2023-08-10 12:20         ` Devin Heitmueller
2023-08-10 12:41           ` Kieran Kunhya
2023-08-10 12:48             ` Kieran Kunhya
2023-08-10 12:58               ` Devin Heitmueller
2023-08-10 13:08                 ` Kieran Kunhya
2023-08-10 15:12                   ` Devin Heitmueller [this message]
2023-08-10 15:53                     ` Kieran Kunhya
2023-07-31 13:38 ` [FFmpeg-devel] [PATCH v4 3/4] mpegtsenc: Add support for output of SCTE-35 streams over TS Devin Heitmueller
2023-07-31 13:38 ` [FFmpeg-devel] [PATCH v4 4/4] bsf: Add new bitstream filter to set SCTE-35 pts_adjustment when reclocking Devin Heitmueller
2023-08-04 11:16 ` [FFmpeg-devel] [PATCH v4 0/4] Add passthrough support for SCTE-35 Devin Heitmueller
2023-08-08 14:31   ` Devin Heitmueller
2023-08-08 15:30     ` Dennis Mungai
2023-09-01  9:29       ` Dennis Mungai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHGibzHQPXH-Eg6_RdYtGZpQnWg33hURAYVyXH+7rPY4k6_C2g@mail.gmail.com \
    --to=devin.heitmueller@ltnglobal.com \
    --cc=dheitmueller@ltnglobal.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    --cc=kierank@obe.tv \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git