* [patch] [media] cx25821: testing the wrong variable
@ 2012-09-29 7:12 Dan Carpenter
2012-09-29 10:52 ` walter harms
2012-10-07 12:49 ` Mauro Carvalho Chehab
0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2012-09-29 7:12 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Leonid V. Fedorenchik, Thomas Meyer, Hans Verkuil, linux-media,
kernel-janitors
->input_filename could be NULL here. The intent was to test
->_filename.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
I'm not totally convinced that using /root/vid411.yuv is the right idea.
diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c b/drivers/media/pci/cx25821/cx25821-video-upstream.c
index 52c13e0..6759fff 100644
--- a/drivers/media/pci/cx25821/cx25821-video-upstream.c
+++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c
@@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev *dev, int channel_select,
}
/* Default if filename is empty string */
- if (strcmp(dev->input_filename, "") == 0) {
+ if (strcmp(dev->_filename, "") == 0) {
if (dev->_isNTSC) {
dev->_filename =
(dev->_pixel_format == PIXEL_FRMT_411) ?
diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
index c8c94fb..d33fc1a 100644
--- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
+++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
@@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev, int channel_select,
}
/* Default if filename is empty string */
- if (strcmp(dev->input_filename_ch2, "") == 0) {
+ if (strcmp(dev->_filename_ch2, "") == 0) {
if (dev->_isNTSC_ch2) {
dev->_filename_ch2 = (dev->_pixel_format_ch2 ==
PIXEL_FRMT_411) ? "/root/vid411.yuv" :
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [patch] [media] cx25821: testing the wrong variable
2012-09-29 7:12 [patch] [media] cx25821: testing the wrong variable Dan Carpenter
@ 2012-09-29 10:52 ` walter harms
2012-09-29 14:32 ` Dan Carpenter
2012-10-07 12:49 ` Mauro Carvalho Chehab
1 sibling, 1 reply; 6+ messages in thread
From: walter harms @ 2012-09-29 10:52 UTC (permalink / raw)
To: Dan Carpenter
Cc: Mauro Carvalho Chehab, Leonid V. Fedorenchik, Thomas Meyer,
Hans Verkuil, linux-media, kernel-janitors
Am 29.09.2012 09:12, schrieb Dan Carpenter:
> ->input_filename could be NULL here. The intent was to test
> ->_filename.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I'm not totally convinced that using /root/vid411.yuv is the right idea.
>
> diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c b/drivers/media/pci/cx25821/cx25821-video-upstream.c
> index 52c13e0..6759fff 100644
> --- a/drivers/media/pci/cx25821/cx25821-video-upstream.c
> +++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c
> @@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev *dev, int channel_select,
> }
>
> /* Default if filename is empty string */
> - if (strcmp(dev->input_filename, "") == 0) {
> + if (strcmp(dev->_filename, "") == 0) {
> if (dev->_isNTSC) {
> dev->_filename =
> (dev->_pixel_format == PIXEL_FRMT_411) ?
> diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> index c8c94fb..d33fc1a 100644
> --- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> +++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> @@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev, int channel_select,
> }
>
> /* Default if filename is empty string */
> - if (strcmp(dev->input_filename_ch2, "") == 0) {
> + if (strcmp(dev->_filename_ch2, "") == 0) {
> if (dev->_isNTSC_ch2) {
> dev->_filename_ch2 = (dev->_pixel_format_ch2 ==
> PIXEL_FRMT_411) ? "/root/vid411.yuv" :
>
In this case stcmp seems a bit of a overkill. A simple
*(dev->_filename_ch2) == 0
should be ok ?
re,
wh
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch] [media] cx25821: testing the wrong variable
2012-09-29 10:52 ` walter harms
@ 2012-09-29 14:32 ` Dan Carpenter
2012-09-29 15:17 ` Ezequiel Garcia
0 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2012-09-29 14:32 UTC (permalink / raw)
To: walter harms
Cc: Mauro Carvalho Chehab, Leonid V. Fedorenchik, Thomas Meyer,
Hans Verkuil, linux-media, kernel-janitors
On Sat, Sep 29, 2012 at 12:52:38PM +0200, walter harms wrote:
>
>
> Am 29.09.2012 09:12, schrieb Dan Carpenter:
> > ->input_filename could be NULL here. The intent was to test
> > ->_filename.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > I'm not totally convinced that using /root/vid411.yuv is the right idea.
> >
> > diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c b/drivers/media/pci/cx25821/cx25821-video-upstream.c
> > index 52c13e0..6759fff 100644
> > --- a/drivers/media/pci/cx25821/cx25821-video-upstream.c
> > +++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c
> > @@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev *dev, int channel_select,
> > }
> >
> > /* Default if filename is empty string */
> > - if (strcmp(dev->input_filename, "") == 0) {
> > + if (strcmp(dev->_filename, "") == 0) {
> > if (dev->_isNTSC) {
> > dev->_filename =
> > (dev->_pixel_format == PIXEL_FRMT_411) ?
> > diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> > index c8c94fb..d33fc1a 100644
> > --- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> > +++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> > @@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev, int channel_select,
> > }
> >
> > /* Default if filename is empty string */
> > - if (strcmp(dev->input_filename_ch2, "") == 0) {
> > + if (strcmp(dev->_filename_ch2, "") == 0) {
> > if (dev->_isNTSC_ch2) {
> > dev->_filename_ch2 = (dev->_pixel_format_ch2 ==
> > PIXEL_FRMT_411) ? "/root/vid411.yuv" :
> >
>
> In this case stcmp seems a bit of a overkill. A simple
> *(dev->_filename_ch2) == 0
> should be ok ?
I prefer strcmp() actually. More readable.
regards,
dan carpenter
>
> re,
> wh
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch] [media] cx25821: testing the wrong variable
2012-09-29 14:32 ` Dan Carpenter
@ 2012-09-29 15:17 ` Ezequiel Garcia
0 siblings, 0 replies; 6+ messages in thread
From: Ezequiel Garcia @ 2012-09-29 15:17 UTC (permalink / raw)
To: Dan Carpenter
Cc: walter harms, Mauro Carvalho Chehab, Leonid V. Fedorenchik,
Thomas Meyer, Hans Verkuil, linux-media, kernel-janitors
On Sat, Sep 29, 2012 at 11:32 AM, Dan Carpenter
<dan.carpenter@oracle.com> wrote:
> On Sat, Sep 29, 2012 at 12:52:38PM +0200, walter harms wrote:
>>
>>
>> Am 29.09.2012 09:12, schrieb Dan Carpenter:
>> > ->input_filename could be NULL here. The intent was to test
>> > ->_filename.
>> >
>> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> > ---
>> > I'm not totally convinced that using /root/vid411.yuv is the right idea.
>> >
>> > diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c b/drivers/media/pci/cx25821/cx25821-video-upstream.c
>> > index 52c13e0..6759fff 100644
>> > --- a/drivers/media/pci/cx25821/cx25821-video-upstream.c
>> > +++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c
>> > @@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev *dev, int channel_select,
>> > }
>> >
>> > /* Default if filename is empty string */
>> > - if (strcmp(dev->input_filename, "") == 0) {
>> > + if (strcmp(dev->_filename, "") == 0) {
>> > if (dev->_isNTSC) {
>> > dev->_filename =
>> > (dev->_pixel_format == PIXEL_FRMT_411) ?
>> > diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
>> > index c8c94fb..d33fc1a 100644
>> > --- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
>> > +++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
>> > @@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev, int channel_select,
>> > }
>> >
>> > /* Default if filename is empty string */
>> > - if (strcmp(dev->input_filename_ch2, "") == 0) {
>> > + if (strcmp(dev->_filename_ch2, "") == 0) {
>> > if (dev->_isNTSC_ch2) {
>> > dev->_filename_ch2 = (dev->_pixel_format_ch2 ==
>> > PIXEL_FRMT_411) ? "/root/vid411.yuv" :
>> >
>>
>> In this case stcmp seems a bit of a overkill. A simple
>> *(dev->_filename_ch2) == 0
>> should be ok ?
>
> I prefer strcmp() actually. More readable.
>
... and by no means an overkill.
First, it's not a hotpath or a core component. So readability is the priority.
Second, strcmp over empty string should be very very cheap (right?).
In some arches it's even some asm code.
"Premature optimization is the root of all evil".
Regards,
Ezequiel.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] [media] cx25821: testing the wrong variable
2012-09-29 7:12 [patch] [media] cx25821: testing the wrong variable Dan Carpenter
2012-09-29 10:52 ` walter harms
@ 2012-10-07 12:49 ` Mauro Carvalho Chehab
2012-10-08 22:21 ` Palash Bandyopadhyay
1 sibling, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2012-10-07 12:49 UTC (permalink / raw)
To: Palash Bandyopadhyay, Sri Deevi
Cc: Dan Carpenter, Leonid V. Fedorenchik, Thomas Meyer, Hans Verkuil,
linux-media, kernel-janitors
Em Sat, 29 Sep 2012 10:12:53 +0300
Dan Carpenter <dan.carpenter@oracle.com> escreveu:
> ->input_filename could be NULL here. The intent was to test
> ->_filename.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I'm not totally convinced that using /root/vid411.yuv is the right idea.
Agreed.
Palash, Sri,
Why do we need those files?
Regards,
Mauro
>
> diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c b/drivers/media/pci/cx25821/cx25821-video-upstream.c
> index 52c13e0..6759fff 100644
> --- a/drivers/media/pci/cx25821/cx25821-video-upstream.c
> +++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c
> @@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev *dev, int channel_select,
> }
>
> /* Default if filename is empty string */
> - if (strcmp(dev->input_filename, "") == 0) {
> + if (strcmp(dev->_filename, "") == 0) {
> if (dev->_isNTSC) {
> dev->_filename =
> (dev->_pixel_format == PIXEL_FRMT_411) ?
> diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> index c8c94fb..d33fc1a 100644
> --- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> +++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> @@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev, int channel_select,
> }
>
> /* Default if filename is empty string */
> - if (strcmp(dev->input_filename_ch2, "") == 0) {
> + if (strcmp(dev->_filename_ch2, "") == 0) {
> if (dev->_isNTSC_ch2) {
> dev->_filename_ch2 = (dev->_pixel_format_ch2 ==
> PIXEL_FRMT_411) ? "/root/vid411.yuv" :
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Cheers,
Mauro
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [patch] [media] cx25821: testing the wrong variable
2012-10-07 12:49 ` Mauro Carvalho Chehab
@ 2012-10-08 22:21 ` Palash Bandyopadhyay
0 siblings, 0 replies; 6+ messages in thread
From: Palash Bandyopadhyay @ 2012-10-08 22:21 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Sri Deevi
Cc: Dan Carpenter, Leonid V. Fedorenchik, Thomas Meyer, Hans Verkuil,
linux-media@vger.kernel.org, kernel-janitors@vger.kernel.org
Mauro,
This is to be able to upstream data from the host to the device to be pushed out. I agree this is not an optimal way of doing things and had discussed with Hans on this last month.
I will be making changes to the entire upstream path and get rid of this mechanism that we have in place here.
Rgds,
Palash
-----Original Message-----
From: Mauro Carvalho Chehab [mailto:mchehab@infradead.org]
Sent: Sunday, October 07, 2012 5:50 AM
To: Palash Bandyopadhyay; Sri Deevi
Cc: Dan Carpenter; Leonid V. Fedorenchik; Thomas Meyer; Hans Verkuil; linux-media@vger.kernel.org; kernel-janitors@vger.kernel.org
Subject: Re: [patch] [media] cx25821: testing the wrong variable
Em Sat, 29 Sep 2012 10:12:53 +0300
Dan Carpenter <dan.carpenter@oracle.com> escreveu:
> ->input_filename could be NULL here. The intent was to test
> ->_filename.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> I'm not totally convinced that using /root/vid411.yuv is the right idea.
Agreed.
Palash, Sri,
Why do we need those files?
Regards,
Mauro
>
> diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream.c
> b/drivers/media/pci/cx25821/cx25821-video-upstream.c
> index 52c13e0..6759fff 100644
> --- a/drivers/media/pci/cx25821/cx25821-video-upstream.c
> +++ b/drivers/media/pci/cx25821/cx25821-video-upstream.c
> @@ -808,7 +808,7 @@ int cx25821_vidupstream_init_ch1(struct cx25821_dev *dev, int channel_select,
> }
>
> /* Default if filename is empty string */
> - if (strcmp(dev->input_filename, "") == 0) {
> + if (strcmp(dev->_filename, "") == 0) {
> if (dev->_isNTSC) {
> dev->_filename =
> (dev->_pixel_format == PIXEL_FRMT_411) ?
> diff --git a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> index c8c94fb..d33fc1a 100644
> --- a/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> +++ b/drivers/media/pci/cx25821/cx25821-video-upstream-ch2.c
> @@ -761,7 +761,7 @@ int cx25821_vidupstream_init_ch2(struct cx25821_dev *dev, int channel_select,
> }
>
> /* Default if filename is empty string */
> - if (strcmp(dev->input_filename_ch2, "") == 0) {
> + if (strcmp(dev->_filename_ch2, "") == 0) {
> if (dev->_isNTSC_ch2) {
> dev->_filename_ch2 = (dev->_pixel_format_ch2 ==
> PIXEL_FRMT_411) ? "/root/vid411.yuv" :
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media"
> in the body of a message to majordomo@vger.kernel.org More majordomo
> info at http://vger.kernel.org/majordomo-info.html
Cheers,
Mauro
Conexant E-mail Firewall (Conexant.Com) made the following annotations
---------------------------------------------------------------------
********************** Legal Disclaimer ****************************
"This email may contain confidential and privileged material for the sole use of the intended recipient. Any unauthorized review, use or distribution by others is strictly prohibited. If you have received the message in error, please advise the sender by reply email and delete the message. Thank you."
**********************************************************************
---------------------------------------------------------------------
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-08 22:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-29 7:12 [patch] [media] cx25821: testing the wrong variable Dan Carpenter
2012-09-29 10:52 ` walter harms
2012-09-29 14:32 ` Dan Carpenter
2012-09-29 15:17 ` Ezequiel Garcia
2012-10-07 12:49 ` Mauro Carvalho Chehab
2012-10-08 22:21 ` Palash Bandyopadhyay
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).