[pvrusb2] initialization of video_gop_size

[Hans Verkuil]

On Sunday 22 June 2008 21:05:43 Dan Bodoh wrote:
> On Sun, Jun 22, 2008 at 1:43 PM, Mike Isely <isely at isely.net> wrote:
> > On Sun, 22 Jun 2008, Dan Bodoh wrote:
> >> Should pvrusb2-encoder.c be setting this value to 15 for NTSC?  Is 
12
> >> a legal value for NTSC?  Or should the default be set correctly in 
the
> >> cx2341x module?
> >
> > The pvrusb2 driver internally is doing what it is doing because, 
well,
> > it's been doing that since basically the beginning of time for the
> > driver.  However that's only there for older kernels and has no 
effect
> 
> I probably got a little too wordy in my question.  I'll be more 
direct:
> 
> (1) Is video_gop_size=12 legal for 30-fps NTSC?  Or should it be 15?
> 
>     There's some evidence that it is not legal, or at least not 
desirable:
>       (A) There's (inactive) code pvrusb2, only active when not using
> cx2341x,  that sets video_gop_size to 15 for NTSC.
>        (B) An old ivtv (0.4.7) on my gentoo box with 2.26.14 kernel,
> which did not use cx2341x, set video_gop_size to 15 for NTSC for a
> pvr-350.
>        (C) Mythtv doesn't handle NTSC with video_gop_size=12 very
> well, but it's ok at 15.
> 
> (2) If video_gop_size should be 15 for NTSC, who is responsible for
> setting it?  mythtv? pvrusb2?  cx2341x?  Because, today, none of these
> are setting it today to 15.
> 
> Presumably it's not an application-level decision (mythtv), since it's
> previously been handled by old drivers.
> 
> So, is this a bug for the cx2341x maintainers or for pvrusb2?

The ivtv and cx18 drivers set it during driver initialization (after 
cx2341x_fill_defaults() is called):

  itv->params.video_gop_size = itv->is_60hz ? 15 : 12;

But I notice that none of the other drivers using cx2341x do so. I've 
CCed the cx88-blackbird and cx23885-417 maintainers as well so that 
they can update their drivers, or at the very least are aware of this.

The use of a GOP size of 12 is (as I understand it) legal but 
non-standard. A GOP size of 15 gives you two I-frames per second, 
similar to the GOP size of 12 for 25 fps formats.

An additional problem I discovered is that the gop_size isn't updated 
after you change from e.g. PAL to NTSC or vice versa. I'm thinking that 
it might be better to have it set to a default of 0, meaning 'use the 
current TV standard to decide on the appropriate GOP size'. But this 
requires a minor API change.

I'll look at this tomorrow and if I haven't come up with a better idea 
I'll propose such a change.

Small fixes to bring cx88, cx23885 and pvrusb2 in line with ivtv/cx18 
should most likely be OK to get into 2.6.26, but an API change is 
definitely 2.6.27.

Regards,

	Hans