Re: question about bt8xx/bttv-audio-hook.c, tvaudio.c

[Hans Verkuil <hverkuil@xs4all.nl>]

On Wed June 6 2012 09:06:23 Julia Lawall wrote:
> The files drivers/media/video/bt8xx/bttv-audio-hook.c and 
> drivers/media/video/tvaudio.c contain a number of occurrences of eg:
> 
> mode |= V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2;
> 
> and
> 
> if (mode & V4L2_TUNER_MODE_MONO)
> 
> (both from tvaudio.c)
> 
> V4L2_TUNER_MODE_LANG1 | V4L2_TUNER_MODE_LANG2 is suspicious because 
> V4L2_TUNER_MODE_LANG1 is 3 and V4L2_TUNER_MODE_LANG2 is 2, so the result 
> is just the same as V4L2_TUNER_MODE_LANG1.  Maybe 
> V4L2_TUNER_MODE_LANG1_LANG2 was intended?
> 
> mode & V4L2_TUNER_MODE_MONO is suspicious because V4L2_TUNER_MODE_MONO is 
> 0.  Maybe & should be ==?
> 
> If & is to be changed to == everywhere, then some new code may need to be 
> constructed to account for V4L2_TUNER_MODE_LANG1_LANG2.  For example, the 
> function tda8425_setmode has ifs for the other values, but not for this 
> one.  On the other hand, the function ta8874z_setmode already uses == (or 
> rather switch), and does not take V4L2_TUNER_MODE_LANG1_LANG2 into 
> account, so perhaps it is not appropriate in this context?

I would have to analyse this more carefully, but the core issue here is that
these drivers mixup the tuner audio reception bitmask flags (V4L2_TUNER_SUB_*)
and the tuner audio modes (V4L2_TUNER_MODE_*, not a bitmask). This happened
regularly in older drivers, and apparently these two are still not fixed.

More info is here:

http://hverkuil.home.xs4all.nl/spec/media.html#vidioc-g-tuner

I can't just replace one define with another, I would need to look carefully
at the code to see what was intended.

If you find more places where this happens, then please let us know. Otherwise
this is something for us to fix.

Regards,

	Hans