On Sun, 2010-11-14 at 11:53 +0100, Hans Verkuil wrote:
> On Sunday, November 14, 2010 11:14:54 Martin Dauskardt wrote:
> > Hi everybody,
> > 
> > just a few comments from an application developer :-)
> > 
> > 
> > > From: Andy Walls <[email protected]>
> >  
> > > That is valid behavior per the V4L2 API:
> > > http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-g-std.html
> > > the driver is allowed to return an errno of EBUSY when it is busy.
> > 
> > Unfortunately the API is not precise enough and allows different handling. 
> > Can you imagine this with a DVB driver? No, once you have a driver for a 
> > DVB 
> > card you can use the card with any DVB application. 
> > But not so with analogue encoder cards: The application needs to know the 
> > specialities of the driver.
> 
> The problem here is that even though the spec clearly stated that drivers
> may return EBUSY applications never heeded this warning.

MythTV does heed it and actually looks for EBUSY.  The problem for
MythTV is what to do about it.

That's where all sorts of funny rules get implemented in the application
to accomplish the task.  The funny rules come from the different methods
described in the API standard for stopping the stream.

If we can cut down on the options in the API for accomplishing the same
action, then there are less chances for interoperability problems
between applications and drivers.  Application programming becomes
simpler.

Hypothetically, even if the only action for stopping a stream was to
close the device and reopen it, that would be unambiguous and
straightforward for application developers.  (Although that option alone
is somewhat inelegant.)

I'm all for converting the read() method drivers to videobuf2, and
making the read() method a secondary way of accessing video streams.



> I hope that once the videobuf2 framework is merged cx18 and possibly ivtv
> can move over to videobuf2 as well. Although this may be wishful thinking
> since it is a huge conversion job. But then cx18/ivtv would finally support
> streaming I/O.

It may not be so wishful for cx18.  I'll need some time to grok the ivtv
buffer handling.  It's a little different.  Then of course I'll need
free time... :)


 
> > > The cx18 driver automatically closing all analog streams (MPEG,
> > > YUV/HM12, PCM, VBI) when someone calls a VIDIOC_S_STD ioctl() on a
> > > device node would be considered a non-compliance with the V4L2 API.  I'm
> > > not sure it would be a desirable feature either.
> > 
> > The cx88-blackbird driver (also cx2341x) internally calls an enoder stop 
> > firmware command (CX2341X_ENC_STOP_CAPTURE) before changing frequency.
> > (But unfortunately, due to a driver bug, it never starts the encoder 
> > again... 
> > but this is a different issue)
> > 
> > There are only a few cases where the driver returns EBUSY on channel 
> > switches. 
> > It depends on a standard change, an input change and switch between 
> > radio/TV. 
> > Instead of doing a lot of checks most applications (mythv and vdr plugin 
> > pvrinput) will handle each channel switch equal.
> > 
> > So we have different methods for changing channels:
> > 
> > 1. stop reading, close, re-open, set frequency/change input, start reading
> > 2. use VIDIOC_STREAMOFF /VIDIOC_STREAMON (buggy at least for ivtv and 
> > pvrusb2)
> > 3. use  V4L2_ENC_CMD_STOP /  V4L2_ENC_CMD_START if the driver supports 
> > these 
> > ioctl (only ivtv, cx18 and hdpvr does)
> > 4. call only VIDIOC_S_FREQUENCY and let the driver do all necessary (only 
> > HVR1300/blackbird has this concept, but unfortunately the driver is so 
> > buggy 
> > that using the encoder on this card is impossible)
> > 
> > I prefer the first method.  This way all internal buffers are cleared, we 
> > start on the new channel with an i-frame and have smooth channel switching 
> > (like a DVB receiver) without any artefacts. So it is really **useful** to 
> > stop read thread and encoder for channel switchings. 
> 
> I agree. The first method will work everywhere and always. The second method
> is suspect due to videobuf bugs in the STREAMOFF implementation :-(


> The third method works fine, but few drivers support it.
>  If someone would be
> willing to work on this, then it can be extended to all drivers that do MPEG.

If few drivers support it, I'd argue let's plan on keeping it's use
limited to old drivers.  Time is probably better spent converting the
old drivers to use videobuf2.


> The last method is probably not possible.
>  It is not a good idea to stop and
> restart as a side-effect of changing a channel since this will interrupt the
> mpeg stream, and will cause issues with time stamping, etc.

Agree.

> > 
> > > Ah, I found the problem in MythTV 0.21 source code.
> > > 
> > > Look at 
> > > 
> > > lib/libmythtv/mpegrecorder.cpp:MpegRecorder::OpenV4L2DeviceAsInput()
> > > 
> > >     if (CardUtil::GetV4LInfo(chanfd, card, driver, version))
> > >     {
> > >         if (driver == "ivtv")
> > >         {
> > >             usingv4l2     = (version >= IVTV_KERNEL_VERSION(0, 8, 0));
> > >             has_v4l2_vbi  = (version >= IVTV_KERNEL_VERSION(0, 3, 8));
> > >             has_buggy_vbi = true;
> > >             requires_special_pause =
> > >                 (version >= IVTV_KERNEL_VERSION(0, 10, 0));
> > >         }
> > >         else
> > >         {
> > >             VERBOSE(VB_IMPORTANT, "\n\nNot ivtv driver??\n\n");
> > >             usingv4l2 = has_v4l2_vbi = true;
> > >             has_buggy_vbi = requires_special_pause = false;
> > >         }
> > >     }
> > > 
> > > Like all modern ivtv driver versions, I'm very confident all cx18 driver
> > > versions require 
> > > 
> > >         requires_special_pause = true;
> > > 
> > > to be set.  Then libmythtv will send the VIDIOC_ENC_CMD,
> > > V4L2_ENC_CMD_STOP that is required.  Although the above code snippet is
> > > from 0.21, I suspect it is still the same in 0.23.
> > > 
> > > I'm not sure what "has_buggy_vbi" means and whether or not it needs to
> > > be set for the cx18 driver.
> > 
> > If I remember right, there is probably something wrong with vbi (closed 
> > captions?) in the ivtv (and probably cx18) kernel driver. This is why some 
> > mythtv guys still prefer the old standalone ivtv driver
> > see change in 02/2008: 
> > http://svn.mythtv.org/trac/changeset/15734/trunk/mythtv/libs/libmythtv/mpegrecorder.cpp
> > "This also updates Hans' VBI bugs warning for ivtv drivers before 0.10.0 
> > and 
> > after 0.10.5 due to new VBI bugs in more recent ivtv drivers."
> 
> While there has been a period where vbi was broken in ivtv, that has been
> fixed about 2 years ago (mid-2008).

The outstanding question posed by the comments in the code is whether
PVR-250/350 VBI is working in modern ivtv drivers with MythTV.

I can test that sometime, when I get the time, but will gladly accept
reports of it working from anyone. ;)
 
> > > VIDIOC_ENC_CMD with V4L2_ENC_CMD_STOP
> > > 
> > > but that is marked as "experimental" in the API document.  close() is
> > > not "experimental". ;)
> > 
> > They were experimental in 2007 when Hans Verkuil introduced them. But now 
> > they 
> > should be a must for every mpeg encoder driver.
> 
> Yeah, it's time to scan the spec for 'experimental' and remove it where that 
> is
> no longer relevant.

Right, the VIDOIOC_ENC_CMD should not be "experimental" anymore.  I
would suggest though that it be marked "not for use in new application
and driver designs".  Probably also add a note to use close() instead of
V4L2_ENC_CMD_STOP for drivers that only support the read() I/O method.

Regards,
Andy


_______________________________________________
ivtv-devel mailing list
[email protected]
http://ivtvdriver.org/mailman/listinfo/ivtv-devel

Reply via email to