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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to