RE: em28xx: fix: some webcams don't have audio inputs

RE: em28xx: fix: some webcams don't have audio inputs

[Devin Heitmueller]

Hello Mauro,

I just noticed this patch:

em28xx: fix: some webcams don't have audio inputs
http://linuxtv.org/hg/v4l-dvb/rev/fe5eeff6644d

I have to wonder what the EM28XX_R00_CHIPCFG contained on this
particular device, since this cause should have already been handled
by the elseif() block on line 507:

} else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) == 0x00) {
    /* The device doesn't have vendor audio at all */
   dev->has_alsa_audio = 0;
   dev->audio_mode.has_audio = 0;
   return 0;
}

On a related note, is there some rationale you can offer as to why you
are committing patches directly into the v4l-dvb mainline without any
peer review, unlike *every* other developer in the linuxtv project?  I
know it may seem redundant to you since you are the person acting on
the PULL requests, but it would provide an opportunity for the other
developers to offer comments on your patches *before* they go into the
mainline.

I for one have had to fix a number of regressions you introduced into
the codebase which may have been avoided if the patches had been peer
reviewed.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: em28xx: fix: some webcams don't have audio inputs

[Mauro Carvalho Chehab]

Em Wed, 5 Aug 2009 14:58:23 -0400
Devin Heitmueller <dheitmueller@kernellabs.com> escreveu:

> Hello Mauro,
> 
> I just noticed this patch:
> 
> em28xx: fix: some webcams don't have audio inputs
> http://linuxtv.org/hg/v4l-dvb/rev/fe5eeff6644d
> 
> I have to wonder what the EM28XX_R00_CHIPCFG contained on this
> particular device, since this cause should have already been handled
> by the elseif() block on line 507:
> 
> } else if ((cfg & EM28XX_CHIPCFG_AUDIOMASK) == 0x00) {
>     /* The device doesn't have vendor audio at all */
>    dev->has_alsa_audio = 0;
>    dev->audio_mode.has_audio = 0;
>    return 0;
> }

Good point. I'll double check. I need one webcam with an integrated mic to be
sure if R00 has the proper value on webcams.

> On a related note, is there some rationale you can offer as to why you
> are committing patches directly into the v4l-dvb mainline without any
> peer review, unlike *every* other developer in the linuxtv project?  I
> know it may seem redundant to you since you are the person acting on
> the PULL requests, but it would provide an opportunity for the other
> developers to offer comments on your patches *before* they go into the
> mainline.

This were already answered on some previous msgs at the ML: hg commits mailing
lists give the opportunity for people to review what were committed at the
staging tree, since every patch is automatically mailbombed to the mailing
list. The mainline tree is my -git. It is delayed over -hg to give opportunity
for people to review the committed patches. Also, I'm not the kind of person
that use to talk to himself. Starting sending pull requests from me to myself
will probably get me a free ticket to a mental care services :-d



Cheers,
Mauro

Direct v4l-dvb master commits

[Hans Verkuil]

On Friday 07 August 2009 18:28:41 Mauro Carvalho Chehab wrote:
> Em Wed, 5 Aug 2009 14:58:23 -0400
> Devin Heitmueller <dheitmueller@kernellabs.com> escreveu:
> > On a related note, is there some rationale you can offer as to why you
> > are committing patches directly into the v4l-dvb mainline without any
> > peer review, unlike *every* other developer in the linuxtv project?  I
> > know it may seem redundant to you since you are the person acting on
> > the PULL requests, but it would provide an opportunity for the other
> > developers to offer comments on your patches *before* they go into the
> > mainline.
> 
> This were already answered on some previous msgs at the ML: hg commits mailing
> lists give the opportunity for people to review what were committed at the
> staging tree, since every patch is automatically mailbombed to the mailing
> list. The mainline tree is my -git. It is delayed over -hg to give opportunity
> for people to review the committed patches. Also, I'm not the kind of person
> that use to talk to himself. Starting sending pull requests from me to myself
> will probably get me a free ticket to a mental care services :-d

I have to say that I really disagree with that. The hg master v4l-dvb tree is
what everyone develops against. So when bad patches go in without having had
the opportunity for a review, then that will affect all of us.

There is also no way to remove such a broken patch from the master tree.
Once it is in it can only be removed by committing a revert patch.

Now, I have no problem with you committing trivial fixes directly into the
master repository, but anything non-trivial should also get the chance to
be reviewed by others. It's not talking to yourself, it's asking for a review
before you commit, just like we all do. Just post a pull request and if
there are no comments after 24 hours, then commit.

Two cases that come to mind that irritated me were the commit of the cx231xx
driver without subdev support out of nowhere a few months ago and your
addition of --get/set-param support in v4l2-ctl just a few days ago which had
several bugs (fixed in my pending v4l-dvb-misc tree).

If these commits were posted as pull requests then I would have reviewed them
first and any issues would have been fixed before they entered the master repo.

There is also the human factor to consider: it can be demotivating (or at
least irritating) if your own pull requests are still queued after several
days and you see the maintainer doing big commits out of the blue at the
same time.

Regards,

	Hans

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG Telecom

Re: Direct v4l-dvb master commits

[hermann pitton]

Am Samstag, den 08.08.2009, 10:55 +0200 schrieb Hans Verkuil:
> On Friday 07 August 2009 18:28:41 Mauro Carvalho Chehab wrote:
> > Em Wed, 5 Aug 2009 14:58:23 -0400
> > Devin Heitmueller <dheitmueller@kernellabs.com> escreveu:
> > > On a related note, is there some rationale you can offer as to why you
> > > are committing patches directly into the v4l-dvb mainline without any
> > > peer review, unlike *every* other developer in the linuxtv project?  I
> > > know it may seem redundant to you since you are the person acting on
> > > the PULL requests, but it would provide an opportunity for the other
> > > developers to offer comments on your patches *before* they go into the
> > > mainline.
> > 
> > This were already answered on some previous msgs at the ML: hg commits mailing
> > lists give the opportunity for people to review what were committed at the
> > staging tree, since every patch is automatically mailbombed to the mailing
> > list. The mainline tree is my -git. It is delayed over -hg to give opportunity
> > for people to review the committed patches. Also, I'm not the kind of person
> > that use to talk to himself. Starting sending pull requests from me to myself
> > will probably get me a free ticket to a mental care services :-d
> 
> I have to say that I really disagree with that. The hg master v4l-dvb tree is
> what everyone develops against. So when bad patches go in without having had
> the opportunity for a review, then that will affect all of us.
> 
> There is also no way to remove such a broken patch from the master tree.
> Once it is in it can only be removed by committing a revert patch.
> 
> Now, I have no problem with you committing trivial fixes directly into the
> master repository, but anything non-trivial should also get the chance to
> be reviewed by others. It's not talking to yourself, it's asking for a review
> before you commit, just like we all do. Just post a pull request and if
> there are no comments after 24 hours, then commit.
> 
> Two cases that come to mind that irritated me were the commit of the cx231xx
> driver without subdev support out of nowhere a few months ago and your
> addition of --get/set-param support in v4l2-ctl just a few days ago which had
> several bugs (fixed in my pending v4l-dvb-misc tree).
> 
> If these commits were posted as pull requests then I would have reviewed them
> first and any issues would have been fixed before they entered the master repo.
> 
> There is also the human factor to consider: it can be demotivating (or at
> least irritating) if your own pull requests are still queued after several
> days and you see the maintainer doing big commits out of the blue at the
> same time.
> 
> Regards,
> 
> 	Hans

Hi Hans,

for the cx231xx I can't tell, but the em28xx webcam stuff will probably
soon have a separate repo before anything is pulled in again.

Your v4l2-ctl fixes will go in without more work for you.
Sorry, I saw too late, that you have a pull request already.

It is just that, that some such cams arrive here by mail, includes
private data of people not related to GNU/Linux, and we can make them
work very quickly.

It is stuff sold in huge masses since 2005, without any support until
now.

Likely we can get them all, but should be no further excuse for not
having them better on a separate tree at first.

With my other rants I'm also finished. I likely won't care for bugs on
Hauppauge/Pinnacle products anymore and will have much more fun with
linux again.

Cheers,
Hermann