On 03/28/2014 10:51 AM, Mikhail Domrachev wrote: > Hi Hans, > > Thank you for comments, I will rework the patch and document the new > event type. > > Let me explain why I created a new thread. > My company is engaged in the monitoring of TV air. All TV channels are > recorded 24/7 for further analysis. But some local TV channels change > the standard over time (SECAM->PAL, PAL->SECAM). So the recording > software must be notified about these changes to set a new standard and > record the picture but not the noise. OK, fair enough. Once I receive the reworked version I'll review again. Expect that you probably will need to make at least one other revision after that as I did not do an in-depth review yet. It might also be a good idea to look at this: http://git.linuxtv.org/hverkuil/media_tree.git/shortlog/refs/heads/saa7134 The patches in that tree convert the saa7134 driver to the videobuf2 framework. This is a huge change that should make the driver a lot more stable and more compliant to the v4l2 specification. If your company is using this driver, then I would recommend that you test this. A pull request was made for this for kernel 3.16 and I tested it quite extensively, but if you can test it as well then that would be very useful. Regards, Hans > > Regards, > Mikhail > > On Fri, 2014-03-28 at 09:37 +0100, Hans Verkuil wrote: >> Hi Mikhail, >> >> Thank you for the patch. However, it does need some work before I can accept it. >> >> First of all, run your patch through scripts/checkpatch.pl to ensure it complies >> to the kernel coding style. >> >> Secondly, split up this single patch in smaller ones: in particular the addition >> of the new event type needs to be in a patch of its own. >> >> Thirdly, you need to document the new event type in the DocBook documentation as >> well. API additions are only accepted if the documentation is updated at the same >> time. >> >> I also wonder why you need a thread to watch for signal changes. It's not wrong, >> but in practice a TV input signal rarely if ever changes format. It can be different >> between different countries or when testing with a signal generator, but the normal >> case is that you are just interested in the current standard, and not how it might >> change over time. That would simplify the code a lot. This is what other drivers >> that implement querystd do. >> >> Regards, >> >> Hans -- 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
