[pvrusb2] [patch] 24xxx capture window cropping support

[Carsten Meier]

Hi,

comments are inlined.

Am Wed, 9 Sep 2009 23:02:35 -0500 (CDT)
schrieb Mike Isely <isely at isely.net>:

> On Tue, 8 Sep 2009, Carsten Meier wrote:
> 
> > Am Tue, 8 Sep 2009 10:37:23 -0500 (CDT)
> > schrieb Mike Isely <isely at isely.net>:
> > 
> > > On Tue, 8 Sep 2009, Carsten Meier wrote:
> > > 
> > > > Am Mon, 07 Sep 2009 22:51:14 +0200
> > > > schrieb Bernhard Geier <geierb at geierb.de>:
> > > > 
> > > > > vdb128 at picaros.org schrieb:
> > > > > > Here below is a patch for the setup: 
> > > > > > Linux 2.6.29.6, 24xxx device, cx25840 decoder, firmware
> > > > > > 2.06.039. 
> > > > > Works! Thank you!
> > > > > 
> > > > > This settings elimiate the black borders at top & bottom:
> > > > > 
> > > > > echo "16x9" > ctl_video_aspect/cur_val
> > > > > echo 428 > ctl_crop_height/cur_val
> > > > > echo 72 > ctl_crop_top/cur_val
> > > > > 
> > > > > Bernhard
> > > > > 
> > > > 
> > > > I don't have the ability to test it yet, because I'm currently
> > > > connected to a satelite dish, but shouldn't the aspect-ratio of
> > > > the stream be automatically calculated from the cropping
> > > > settings?
> > > 
> > > The driver does not internally attempt to set that control, and I 
> > > seriously doubt that the mpeg encoder would do that on its own
> > > either.  
> > > 
> > >   -Mike
> > > 
> > 
> > The video aspect control should not change, because it specifies the
> > aspect ratio of the input signal, which remains constant. But the
> > aspect ratio of the MPEG stream should be adapted accordingly to
> > preserve pixel dimensions. E.g.: A 4:3 letterboxed TV broadcast
> > (with square pixels) contains an 2:1 image with black bars at the
> > top and the bottom. If I set the crop-controls to remove the black
> > bars, shouldn't the MPEG stream have an aspect ratio of 2:1? (A
> > value that is not even settable through controls, because of the
> > enum) Otherwise the square pixels get streched.
> 
> If I understand what you are saying here, then you are suggesting
> that:
> 
> 1. The driver always assumes that the incoming video frame is 4:3
> 
No, I meant it a little bit different:

The driver takes the value of the video aspect control as the aspect
ratio of the video input This is what the user usually wants to do,
goes well with DVD-players, TV etc. The user don't want to squeeze
*output*, she/he wants the aspect ratio to fit to the *input* material.
Bot are equal if there is no cropping in effect. Here the enum is quite
right, because there aren't any "odd" values other than 4:3, 16:9, and
2.21:1.

> 2. The driver calculates the apparent effect on the aspect ratio from 
> any cropping operation.
> 

All right...

> 3. Based on the calculation from (2), then the driver should SET 
> video_aspect by itself.
> 

The driver shouldn't modify the control, because it is a user setting
and it would cause a cycle of always changing aspect-ratios. Instead
the values from the calculations from (2) should be passed to the
internal MPEG-encoder-API and leave the control untouched.

> I have three problems with that:
> 
> First, I don't think it's fair for the driver to assume that the 
> incoming video is always 4:3.  That reality really depends on how the 
> device is being used.  One obvious case I can come up with is if
> someone were using this to capture anamorphic video (from, say, a DVD
> player).
> 
> Second, any such calculation is extra magic that I don't think
> belongs in the driver (though I could probably be swayed on this
> point).
> 
> Third, the mpeg encoder does not allow for arbitrary setting of the 
> aspect ratio value.  The reason why you don't see 2:1 as one of the 
> choices is because that enumerated set of values is due entirely to
> the mpeg encoder's own internal API.  It really is an enumeration on
> the inside.  We simply don't have any other choices besides the ones
> that are listed.  Thus what is the driver supposed to do now if the 
> calculated aspect ratio doesn't match one of the choices?  Pick the 
> closest?  Define "closest" (remember, we're dealing with 2 dimensions
> so it isn't a linear relation).  And if the driver's idea of closest
> isn't liked by the user, then what?
> 

Hmmm... This is a real problem... Is it a limitation of the hardware
or the MPEG format? If not, I think the internal API isn't well defined.


> What the driver does right now is that it makes no assumptions about
> the incoming aspect ratio; after all, to assume something here would
> really be just "making up" additional input information out of
> nothing.  The driver does provide a knob that userspace can twiddle
> to set the *output* aspect ratio (i.e. video_aspect).  Given that (1)
> userspace is in control of the cropping, (2) userspace is in control
> of the setting of the aspect ratio, and (3) only the user *really*
> knows what the aspect ratio is of the incoming signal, then any such
> calculation and adjustment probably really belongs in userspace.
> 
> Then again, perhaps I'm not following you here?...
> 
>   -Mike
> 
> 

I guess the most common use case for cropping is to remove
letterbox-bars from the image. Because TV-stations often use zoom
factors (at least for older movies) which produce images other than
4:3, 16:9, and 2.21:1 it is desirable to have this working. Otherwise I
have to run e.g. ProjectX to correct the aspect ratio. (Hmmm, I always
run it to remove commercials, so even to current patches are highly
welcome :) )

OK, just my thoughts on the cropping-thingy... Will the patches go to
the mainline-kernel?

Cheers,
Carsten