RE: [patch] [media] cx25821: testing the wrong variable

2012-10-08 Thread Palash Bandyopadhyay
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:mche...@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-janit...@vger.kernel.org
Subject: Re: [patch] [media] cx25821: testing the wrong variable

Em Sat, 29 Sep 2012 10:12:53 +0300
Dan Carpenter dan.carpen...@oracle.com escreveu:

 -input_filename could be NULL here.  The intent was to test 
 -_filename.
 
 Signed-off-by: Dan Carpenter dan.carpen...@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 majord...@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. 

** 

-

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] [media] cx25821: testing the wrong variable

2012-10-07 Thread Mauro Carvalho Chehab
Em Sat, 29 Sep 2012 10:12:53 +0300
Dan Carpenter dan.carpen...@oracle.com escreveu:

 -input_filename could be NULL here.  The intent was to test
 -_filename.
 
 Signed-off-by: Dan Carpenter dan.carpen...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html




Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] [media] cx25821: testing the wrong variable

2012-09-29 Thread walter harms


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.carpen...@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
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] [media] cx25821: testing the wrong variable

2012-09-29 Thread Dan Carpenter
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.carpen...@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 majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [patch] [media] cx25821: testing the wrong variable

2012-09-29 Thread Ezequiel Garcia
On Sat, Sep 29, 2012 at 11:32 AM, Dan Carpenter
dan.carpen...@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.carpen...@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.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html