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
