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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 9 Jun 2012, Hans Verkuil wrote:

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.

No, I explicitly looked, but only found the problem in these two files.

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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux