Re: [GIT PULL] ViewCast O820E capture support added

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri September 14 2012 04:09:42 Mauro Carvalho Chehab wrote:
> Em 13-09-2012 21:59, Mauro Carvalho Chehab escreveu:
> > Em Thu, 13 Sep 2012 20:23:42 -0300
> > Mauro Carvalho Chehab <mchehab@xxxxxxxxxx> escreveu:
> > 
> >> Em 13-09-2012 20:19, Mauro Carvalho Chehab escreveu:
> >>> Em Sat, 18 Aug 2012 11:48:52 -0400
> >>> Steven Toth <stoth@xxxxxxxxxxxxxx> escreveu:
> >>>
> >>>> Mauro, please read below, a new set of patches I'm submitting for merge.
> >>>>
> >>>> On Thu, Aug 16, 2012 at 2:49 PM, Hans Verkuil <hverkuil@xxxxxxxxx> wrote:
> >>>>> On Thu August 16 2012 19:39:51 Steven Toth wrote:
> >>>>>>>> So, I've ran v4l2-compliance and it pointed out a few things that I've
> >>>>>>>> fixed, but it also does a few things that (for some reason) I can't
> >>>>>>>> seem to catch. One particular test is on (iirc) s_fmt. It attempts to
> >>>>>>>> set ATSC but by ioctl callback never receives ATSC in the norm/id arg,
> >>>>>>>> it actually receives 0x0. This feels more like a bug in the test.
> >>>>>>>> Either way, I have some if (std & ATSC) return -EINVAL, but it still
> >>>>>>>> appears to fail the test.
> >>>>>>
> >>>>>> Oddly enough. If I set tvnorms to something valid, then compliance
> >>>>>> passes but gstreamer
> >>>>>> fails to run, looks like some kind of confusion about either the
> >>>>>> current established
> >>>>>> norm, or a failure to establish a norm.
> >>>>>>
> >>>>>> For the time being I've set tvnorms to 0 (with a comment) and removed
> >>>>>> current_norm.
> >>>>>
> >>>>> Well, this needs to be sorted, because something is clearly amiss.
> >>>>
> >>>> Agreed. I just can't see what's wrong. I may need your advise /
> >>>> eyeballs on this. I'd be willing to provide logs that show gstreamer
> >>>> accessing the driver and exiting. It needs fixed, I've tried, I just
> >>>> can't see why gstreamer fails.
> >>>>
> >>>> On the main topic of merge.... As promised, I spent quite a bit of
> >>>> time this week reworking the code based on the feedback. I also
> >>>> flattened all of these patches into a single patchset and upgraded to
> >>>> the latest re-org tree.
> >>>>
> >>>> The source notes describe in a little more detail the major changes:
> >>>> http://git.kernellabs.com/?p=stoth/media_tree.git;a=commit;h=f295dd63e2f7027e327daad730eb86f2c17e3b2c
> >>>>
> >>>> Mauro, so, I hereby submit for your review/merge again, the updated
> >>>> patchset. *** Please comment. ***
> >>>
> >>> I'll comment patch by patch. Let's hope the ML will get this email. Not sure,
> >>> as it tends to discard big emails like that.
> >>>
> >>> This is the comment of patch 1/4.
> >>>
> >>
> >> Patch 2 is trivial. It is obviously OK.
> >>
> >> Patch 3 also looked OK on my eyes.
> > 
> > Patch 4 will very likely be discarded by vger server, if everything is
> > added there. So, I'll drop the parts that weren't commented.
> > 
> > Anyway:
> > 
> >> Subject: [media] vc8x0: Adding support for the ViewCast O820E Capture Card.
> >> Cc: Linux Media Mailing List <linux-media@xxxxxxxxxxxxxxx>
> >>
> >> A dual channel 1920x1080p60 PCIe x4 capture card, two DVI
> >> inputs capable of capturing DVI/HDMI, Component, Svideo, Composite
> >> and some VGA resolutions.
> > ...
> > 


> >> diff --git a/drivers/media/pci/vc8x0/vc8x0-display.c b/drivers/media/pci/vc8x0/vc8x0-display.c
> > 
> >> +struct letter_t {
> >> +	u8 *ptr;
> >> +	u8 data[8];
> >> +} charset[] = {
> >> + /* ' ' */ [0x20] = { 0, { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 }, },
> >> + /* 00000000 */
> >> + /* 00000000 */
> >> + /* 00000000 */
> >> + /* 00000000 */
> >> + /* 00000000 */
> >> + /* 00000000 */
> >> + /* 00000000 */
> >> + /* 00000000 */
> >> + /* '!' */ [0x21] = { 0, { 0x04, 0x04, 0x04, 0x04, 0x00, 0x00, 0x04, 0x00 }, },
> >> + /* 00000100 */
> >> + /* 00000100 */
> >> + /* 00000100 */
> >> + /* 00000100 */
> >> + /* 00000000 */
> >> + /* 00000000 */
> >> + /* 00000100 */
> >> + /* 00000000 */
> > 
> > Charset???? No, please! If you really need a charset, take a look at the
> > vivi driver. It uses an already-existent Kernel charset. See:
> > 
> > 	static int __init vivi_init(void)
> > 	{
> > 		const struct font_desc *font = find_font("VGA8x16");
> > 
> > Not sure about the rest of the code here at vc8x0-display.c, but maybe you'll
> > find a similar code to it already coded. Where do you use it?

I think all this is just for debugging and should just be removed. It renders
debugging text in the frame.

> > diff --git a/drivers/media/pci/vc8x0/vc8x0-video.c b/drivers/media/pci/vc8x0/vc8x0-video.c
> 

> > +static int vc8x0_video_generate_osd(struct vc8x0_dma_channel *channel, u8 *dst)
> > +{
> > +#if 1
> > +	return 0;
> > +#else
> > +	/* Do some text rendering */
> > +	struct vc8x0_format *fmt = channel->ad7441_ctx.detected_fmt;
> > +	unsigned char tmp[256];
> > +	int ret;
> > +
> > +	ret = vc8x0_display_render_reset(&channel->display_ctx, dst,
> > +		channel->fmt->width);
> > +	if (ret < 0)
> > +		return ret;
> 
> Hmm... Are you using the *-display.c code for OSD? Not sure if it is
> a good idea to handle it like that.
> 
> Hans,
> 
> What do you think?
> 
> Yet, the code here is commented, but there's a hole driver there in order
> to implement OSD display, just bloating the driver's code... 

I think it can all be removed completely. It's not a real OSD, it just
renders text in a captured frame.

> > +static int vc8x0_log_status(struct file *file, void *priv)
> > +{
> > +	struct vc8x0_dma_channel *channel = ((struct vc8x0_fh *)priv)->channel;
> > +	struct vc8x0_dev *dev = channel->dev;
> > +
> > +	v4l2_subdev_call(channel->sd_adv7441a, core, log_status);
> > +	v4l2_subdev_call(dev->dma_channel[DMA_CHANNEL9].sd_pcm3052,
> > +		core, log_status);
> > +
> > +	return 0;
> > +}
> 
> I think this is only available with advanced debug.

No, this is always available. Only g/s_dbg_register are under advanced debug.

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


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux