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".
next prev parent 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