diff mbox

[FFmpeg-devel,1/4] V14 - Adding SCTE-35 CUI codec

Message ID 1476837390-20225-2-git-send-email-carlos@ccextractor.org
State Accepted
Headers show

Commit Message

Carlos Fernandez Sanz Oct. 19, 2016, 12:36 a.m. UTC
From: Carlos Fernandez <carlos@ccextractor.org>

Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
---
 libavcodec/avcodec.h    | 1 +
 libavcodec/codec_desc.c | 6 ++++++
 2 files changed, 7 insertions(+)

Comments

Michael Niedermayer Oct. 20, 2016, 6:03 p.m. UTC | #1
On Tue, Oct 18, 2016 at 05:36:27PM -0700, Carlos Fernandez Sanz wrote:
> From: Carlos Fernandez <carlos@ccextractor.org>
> 
> Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
> ---
>  libavcodec/avcodec.h    | 1 +
>  libavcodec/codec_desc.c | 6 ++++++
>  2 files changed, 7 insertions(+)

Reviewed-by: Michael
LGTM

[...]
Marton Balint Oct. 21, 2016, 7:05 p.m. UTC | #2
On Thu, 20 Oct 2016, Michael Niedermayer wrote:

> On Tue, Oct 18, 2016 at 05:36:27PM -0700, Carlos Fernandez Sanz wrote:
>> From: Carlos Fernandez <carlos@ccextractor.org>
>>
>> Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
>> ---
>>  libavcodec/avcodec.h    | 1 +
>>  libavcodec/codec_desc.c | 6 ++++++
>>  2 files changed, 7 insertions(+)
>
> Reviewed-by: Michael
> LGTM

Pushed with a minor whitespace fix.

Regards,
Marton
Kieran Kunhya Oct. 22, 2016, 12:17 a.m. UTC | #3
On Fri, 21 Oct 2016, 20:05 Marton Balint, <cus@passwd.hu> wrote:

>
> On Thu, 20 Oct 2016, Michael Niedermayer wrote:
>
> > On Tue, Oct 18, 2016 at 05:36:27PM -0700, Carlos Fernandez Sanz wrote:
> >> From: Carlos Fernandez <carlos@ccextractor.org>
> >>
> >> Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
> >> ---
> >>  libavcodec/avcodec.h    | 1 +
> >>  libavcodec/codec_desc.c | 6 ++++++
> >>  2 files changed, 7 insertions(+)
> >
> > Reviewed-by: Michael
> > LGTM
>
> Pushed with a minor whitespace fix.
>
> Regards,
> Marton
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


So all the objections to this patchset are now irrelevant are they?

What a shameful way to run an Open Source project.

Kieran

>
>
Carlos Fernandez Sanz Oct. 22, 2016, 12:41 a.m. UTC | #4
On Fri, Oct 21, 2016 at 5:17 PM, Kieran Kunhya <kierank@obe.tv> wrote:
>
> So all the objections to this patchset are now irrelevant are they?

Just curious - what are the objections to this patchset and where were
they raised? I don't think there's _anything_ posted in this mailing
list that hasn't been corrected or replied to. That's why we're at V14
already.

If there's other places I should have been keeping track of but
didn't, I apologize.


Carlos
Rostislav Pehlivanov Oct. 22, 2016, 4:59 a.m. UTC | #5
On 22 October 2016 at 01:17, Kieran Kunhya <kierank@obe.tv> wrote:

> On Fri, 21 Oct 2016, 20:05 Marton Balint, <cus@passwd.hu> wrote:
>
> >
> > On Thu, 20 Oct 2016, Michael Niedermayer wrote:
> >
> > > On Tue, Oct 18, 2016 at 05:36:27PM -0700, Carlos Fernandez Sanz wrote:
> > >> From: Carlos Fernandez <carlos@ccextractor.org>
> > >>
> > >> Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
> > >> ---
> > >>  libavcodec/avcodec.h    | 1 +
> > >>  libavcodec/codec_desc.c | 6 ++++++
> > >>  2 files changed, 7 insertions(+)
> > >
> > > Reviewed-by: Michael
> > > LGTM
> >
> > Pushed with a minor whitespace fix.
> >
> > Regards,
> > Marton
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
> So all the objections to this patchset are now irrelevant are they?
>
> What a shameful way to run an Open Source project.
>
> Kieran
>
> >
> >
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>


That patch has been posted on the mailing list since July. You didn't reply
to any patch to say why you think it's a bad idea. You just said that it's
inappropriate once on IRC and didn't explain much as to why. You can't
really expect to convince someone like that.
The guy had to go through 14 versions to get something acceptable, which is
one of the most I've seen, and the reviewers did have to do a lot of work
to make it look fine. And I did look at the patch too and found nothing
really wrong with it. In fact SMPTE KLV is implemented in a similar way.

An open source project accepts a well reviewed patch, how is that shameful?
Also mature projects are either dead or no one really uses/works on them
willingly.
Marton Balint Oct. 22, 2016, 10:01 a.m. UTC | #6
On Sat, 22 Oct 2016, Kieran Kunhya wrote:

> On Fri, 21 Oct 2016, 20:05 Marton Balint, <cus@passwd.hu> wrote:
>
>>
>> On Thu, 20 Oct 2016, Michael Niedermayer wrote:
>>
>> > On Tue, Oct 18, 2016 at 05:36:27PM -0700, Carlos Fernandez Sanz wrote:
>> >> From: Carlos Fernandez <carlos@ccextractor.org>
>> >>
>> >> Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
>> >> ---
>> >>  libavcodec/avcodec.h    | 1 +
>> >>  libavcodec/codec_desc.c | 6 ++++++
>> >>  2 files changed, 7 insertions(+)
>> >
>> > Reviewed-by: Michael
>> > LGTM
>>
>> Pushed with a minor whitespace fix.
>>
>> Regards,
>> Marton
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel@ffmpeg.org
>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>
> So all the objections to this patchset are now irrelevant are they?

No they are not, but in this case the people objecting failed to 
specifically explain what is wrong with the patch and how can it be 
improved to be acceptable.

BTW I only pushed patch 1 & 2, which I find even less questionable.

Regards,
Marton
Kieran Kunhya Oct. 22, 2016, 11:15 a.m. UTC | #7
On Sat, 22 Oct 2016, 06:13 Rostislav Pehlivanov, <atomnuker@gmail.com>
wrote:

> On 22 October 2016 at 01:17, Kieran Kunhya <kierank@obe.tv> wrote:
>
> > On Fri, 21 Oct 2016, 20:05 Marton Balint, <cus@passwd.hu> wrote:
> >
> > >
> > > On Thu, 20 Oct 2016, Michael Niedermayer wrote:
> > >
> > > > On Tue, Oct 18, 2016 at 05:36:27PM -0700, Carlos Fernandez Sanz
> wrote:
> > > >> From: Carlos Fernandez <carlos@ccextractor.org>
> > > >>
> > > >> Signed-off-by: Carlos Fernandez <carlos@ccextractor.org>
> > > >> ---
> > > >>  libavcodec/avcodec.h    | 1 +
> > > >>  libavcodec/codec_desc.c | 6 ++++++
> > > >>  2 files changed, 7 insertions(+)
> > > >
> > > > Reviewed-by: Michael
> > > > LGTM
> > >
> > > Pushed with a minor whitespace fix.
> > >
> > > Regards,
> > > Marton
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel@ffmpeg.org
> > > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> >
> > So all the objections to this patchset are now irrelevant are they?
> >
> > What a shameful way to run an Open Source project.
> >
> > Kieran
> >
> > >
> > >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
>
>
> That patch has been posted on the mailing list since July. You didn't reply
> to any patch to say why you think it's a bad idea. You just said that it's
> inappropriate once on IRC and didn't explain much as to why. You can't
> really expect to convince someone like that.
> The guy had to go through 14 versions to get something acceptable, which is
> one of the most I've seen, and the reviewers did have to do a lot of work
> to make it look fine. And I did look at the patch too and found nothing
> really wrong with it. In fact SMPTE KLV is implemented in a similar way.
>
>
> An open source project accepts a well reviewed patch, how is that shameful?
> Also mature projects are either dead or no one really uses/works on them
> willingly.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


http://ffmpeg.org/pipermail/ffmpeg-devel/2016-October/200644.html

The fact that the timestamps are unusable was never addressed. Just because
KLV is like that, doesn't suddenly make two wrongs into a right.

Getting patches merged by sheer attrition is a shameful way of "running" an
open source project.

Kieran
Carlos Fernandez Sanz Oct. 22, 2016, 7:38 p.m. UTC | #8
On Sat, Oct 22, 2016 at 4:15 AM, Kieran Kunhya <kierank@obe.tv> wrote:
>
> Getting patches merged by sheer attrition is a shameful way of "running" an
> open source project.

I think ignoring follow-up questions from the patch submitter, like
you are doing with mine, it's plain rude.

Basically your position is "SCTE-35 doesn't fit into ffmpeg's model
therefore it cannot be supported, ever, and the industry professionals
that want SCTE-35 just need to find a different program". Did I sum it
up correctly?

Really, if there's a problem with my implementation -totally possible-
I'm more than happy to discuss and iterate. And of course I won't take
offense if you send a patch over it or a total rewrite that is better
than mine.

But whining about the fact that my patch has been -partially- merged
even though you wanted to veto it "just because" it's childish.

Carlos
diff mbox

Patch

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index d72ee07..e68dd93 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -631,6 +631,7 @@  enum AVCodecID {
     AV_CODEC_ID_FIRST_UNKNOWN = 0x18000,           ///< A dummy ID pointing at the start of various fake codecs.
     AV_CODEC_ID_TTF = 0x18000,
 
+    AV_CODEC_ID_SCTE_35,///< Contain timestamp estimated through PCR of program stream.
     AV_CODEC_ID_BINTEXT    = 0x18800,
     AV_CODEC_ID_XBIN,
     AV_CODEC_ID_IDF,
diff --git a/libavcodec/codec_desc.c b/libavcodec/codec_desc.c
index 24948ca..2612215 100644
--- a/libavcodec/codec_desc.c
+++ b/libavcodec/codec_desc.c
@@ -2964,6 +2964,12 @@  static const AVCodecDescriptor codec_descriptors[] = {
         .long_name = NULL_IF_CONFIG_SMALL("binary data"),
         .mime_types= MT("application/octet-stream"),
     },
+    {
+        .id        = AV_CODEC_ID_SCTE_35,
+        .type      = AVMEDIA_TYPE_DATA,
+        .name      = "scte_35",
+        .long_name = NULL_IF_CONFIG_SMALL("SCTE 35 Message Queue"),
+    },
 
     /* deprecated codec ids */
 };