public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
From: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
To: Hans Verkuil <hverkuil@xs4all.nl>
Cc: Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Keiichi Watanabe <keiichiw@chromium.org>,
	linux-media@vger.kernel.org,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	linux-kernel@vger.kernel.org, tfiga@chromium.org,
	jcliang@chromium.org, shik@chromium.org
Subject: Re: [PATCH] media: vivid: Support 480p for webcam capture
Date: Mon, 8 Oct 2018 15:23:48 -0300	[thread overview]
Message-ID: <20181008152348.7ef6d77e@coco.lan> (raw)
In-Reply-To: <00b0a8af-b7a5-cb49-0684-0fd0efefa196@xs4all.nl>

Em Mon, 8 Oct 2018 19:53:38 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 10/08/2018 07:03 PM, Mauro Carvalho Chehab wrote:
> > Em Wed, 3 Oct 2018 12:14:22 +0100
> > Kieran Bingham <kieran.bingham@ideasonboard.com> escreveu:
> >   
> >>> @@ -75,6 +76,8 @@ static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = {
> >>>  	{  1, 5 },
> >>>  	{  1, 10 },
> >>>  	{  1, 15 },
> >>> +	{  1, 15 },
> >>> +	{  1, 25 },    
> > 
> > As the code requires that VIVID_WEBCAM_IVALS would be twice the number
> > of resolutions, I understand why you're doing that.
> >   
> >> But won't this add duplicates of 25 and 15 FPS to all the frame sizes
> >> smaller than 1280,720 ? Or are they filtered out?  
> > 
> > However, I agree with Kieran: looking at the code, it sounds to me that
> > it will indeed duplicate 1/15 and 1/25 intervals.  
> 
> Oops, I missed this comment. Yes, you'll get duplicates which should be
> avoided.
> 
> > 
> > I suggest add two other intervals there, like:
> > 	12.5 fps and 29.995 fps, e. g.:  
> 
> 29.995 is never used by webcams.
> 
> > 
> > static const struct v4l2_fract webcam_intervals[VIVID_WEBCAM_IVALS] = {
> >         {  1, 1 },
> >         {  1, 2 },
> >         {  1, 4 },
> >         {  1, 5 },
> >         {  1, 10 },
> >         {  1, 15 },
> > 	{  2, 50 },
> >         {  1, 25 },
> >         {  1, 30 },
> >         {  1, 40 },
> >         {  1, 50 },
> > 	{  1001, 30000 },
> >         {  1, 60 },
> > };
> > 
> > Provided, of course, that vivid would support producing images
> > at fractional rate. I didn't check. If not, then simply add
> > 1/20 and 1/40.  
> 
> vivid can do fractional rates (it does support this for the TV input),
> but 29.995 makes no sense for a webcam. 

Yes, I know.

> So 1/20 and 1/40 seems the
> right approach.

I would have 1/12.5 at least. I have some webcams here whose seem to
use things like that under bad light, and it sounds interesting to
have at least one fraction that doesn't start with "1", in order to
be sure that camera apps are doing the right thing.

> >> Now the difficulty is adding smaller frame rates (like 1,1, 1,2) would
> >> effect/reduce the output rates of the larger frame sizes, so how about
> >> adding some high rate support (any two from 1/{60,75,90,100,120}) instead?  
> > 
> > Last week, I got a crash with vivid running at 30 fps, while running an 
> > event's race code, on a i7core (there, the code was switching all video
> > controls while subscribing/unsubscribing events). The same code worked
> > with lower fps.  
> 
> If you have a stack trace, then let me know.

See at the end.

I intend to do further tests when I have some time.

> 
> > While I didn't have time to debug it yet, I suspect that it has to do
> > with the time spent to produce a frame on vivid. So, while it would be
> > nice to have high rate support, I'm not sure if this is doable. It may,
> > but perhaps we need to disable some possible video output formats, as some
> > types may consume more time to build frames.  
> 
> In the end that depends on the CPU and what else is running. You'll know quickly
> enough if the CPU isn't fast enough to support a format. Although it shouldn't
> crash, of course.

Yes, but on this case, it caused an OOPS (with KASAN enabled).

I was running the stress test on one VT while using qv4l2 to stream.

When I changed the resolution, it caused the OOPS.

This is one example.

I ran the race test first. It placed all controls at some random state
(only issued VIDIOC_EXT_CTRLS - kept everything else on default).

when asked qv4l2 to start streaming (5fps), got this:


[348569.866967] BUG: unable to handle kernel paging request at ffffc90303ff73b5
[348569.867070] PGD 406ee8067 P4D 406ee8067 PUD 0 
[348569.867081] Oops: 0002 [#1] SMP KASAN
[348569.867089] CPU: 2 PID: 4365 Comm: vivid-000-vid-c Tainted: G    B             4.19.0-rc1+ #3
[348569.867098] Hardware name:  /NUC5i7RYB, BIOS RYBDWi35.86A.0364.2017.0511.0949 05/11/2017
[348569.867113] RIP: 0010:tpg_print_str_6+0x241/0x960 [v4l2_tpg]
[348569.867122] Code: 24 18 e9 a5 01 00 00 84 c9 0f 84 48 03 00 00 40 84 ed 48 8d 7b 15 be 02 00 00 00 0f 88 cb 06 00 00 e8 a3 cd 2f ec 48 8d 7b 17 <66> 44 89 73 15 e8 b5 c7
 2f ec 44 88 6b 17 40 f6 c5 40 48 8d 7b 12
[348569.867136] RSP: 0018:ffff88024b6cf8c8 EFLAGS: 00010246
[348569.867144] RAX: fffff520607fee77 RBX: ffffc90303ff73a0 RCX: ffffffffc10c22cd
[348569.867152] RDX: 0000000000000001 RSI: 0000000000000002 RDI: ffffc90303ff73b7
[348569.867162] RBP: 0000000000000000 R08: fffff520607fee77 R09: fffff520607fee77
[348569.867170] R10: 0000000000000001 R11: fffff520607fee76 R12: ffff88024b6cfcf1
[348569.867179] R13: 0000000000000010 R14: 0000000000001010 R15: 00000000000000ea
[348569.867189] FS:  0000000000000000(0000) GS:ffff880407300000(0000) knlGS:0000000000000000
[348569.867198] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[348569.867206] CR2: ffffc90303ff73b5 CR3: 0000000346c0d004 CR4: 00000000003606e0
[348569.867214] Call Trace:
[348569.867227]  tpg_gen_text+0x258/0x2b0 [v4l2_tpg]
[348569.867249]  vivid_fillbuff+0x1e9b/0x30b0 [vivid]
[348569.867261]  ? __list_add_valid+0x29/0x90
[348569.867271]  ? __switch_to+0x345/0x700
[348569.867281]  ? osq_unlock+0x6b/0xf0
[348569.867302]  ? vivid_grab_controls+0x60/0x60 [vivid]
[348569.867312]  ? del_timer_sync+0x3e/0x50
[348569.867320]  ? schedule_timeout+0x234/0x4e0
[348569.867330]  ? mutex_lock+0xbd/0xc0
[348569.867337]  ? mutex_lock+0xbd/0xc0
[348569.867345]  ? __mutex_lock_slowpath+0x10/0x10
[348569.867365]  ? vivid_thread_vid_cap+0x5b6/0xf20 [vivid]
[348569.867385]  vivid_thread_vid_cap+0x5b6/0xf20 [vivid]
[348569.867396]  ? __sched_text_start+0x8/0x8
[348569.867404]  ? __wake_up_common+0x9c/0x230
[348569.867413]  ? __kthread_parkme+0x77/0x90
[348569.867432]  ? vivid_fillbuff+0x30b0/0x30b0 [vivid]
[348569.867440]  kthread+0x1ac/0x1d0
[348569.867448]  ? kthread_create_worker_on_cpu+0xc0/0xc0
[348569.867458]  ret_from_fork+0x1f/0x30
[348569.867465] Modules linked in: vivid videobuf2_dma_contig v4l2_tpg v4l2_dv_timings videobuf2_v4l2 videobuf2_vmalloc videobuf2_memops videobuf2_common v4l2_common videode
v media xt_CHECKSUM iptable_mangle ipt_MASQUERADE iptable_nat nf_nat_ipv4 nf_nat xt_conntrack nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 libcrc32c tun bridge stp llc ebtable
_filter ebtables ip6table_filter ip6_tables bluetooth rfkill ecdh_generic snd_hda_codec_hdmi i915 snd_hda_intel snd_usb_audio snd_hda_codec intel_rapl snd_usbmidi_lib x86_pk
g_temp_thermal snd_hda_core i2c_algo_bit snd_hwdep intel_powerclamp coretemp snd_pcm snd_seq_midi drm_kms_helper crct10dif_pclmul snd_seq_midi_event crc32_pclmul snd_rawmidi
 ghash_clmulni_intel intel_cstate snd_seq intel_uncore drm intel_rapl_perf snd_seq_device snd_timer e1000e snd ptp mei_me
[348569.867574]  video mei soundcore pps_core lpc_ich fuse binfmt_misc kvm_intel kvm irqbypass crc32c_intel [last unloaded: videobuf2_memops]
[348569.867597] CR2: ffffc90303ff73b5
[348569.867604] ---[ end trace b85f80398f88914d ]---
[348569.867615] RIP: 0010:tpg_print_str_6+0x241/0x960 [v4l2_tpg]
[348569.867624] Code: 24 18 e9 a5 01 00 00 84 c9 0f 84 48 03 00 00 40 84 ed 48 8d 7b 15 be 02 00 00 00 0f 88 cb 06 00 00 e8 a3 cd 2f ec 48 8d 7b 17 <66> 44 89 73 15 e8 b5 c7
 2f ec 44 88 6b 17 40 f6 c5 40 48 8d 7b 12
[348569.867639] RSP: 0018:ffff88024b6cf8c8 EFLAGS: 00010246
[348569.867647] RAX: fffff520607fee77 RBX: ffffc90303ff73a0 RCX: ffffffffc10c22cd
[348569.867656] RDX: 0000000000000001 RSI: 0000000000000002 RDI: ffffc90303ff73b7
[348569.867665] RBP: 0000000000000000 R08: fffff520607fee77 R09: fffff520607fee77
[348569.867674] R10: 0000000000000001 R11: fffff520607fee76 R12: ffff88024b6cfcf1
[348569.867682] R13: 0000000000000010 R14: 0000000000001010 R15: 00000000000000ea
[348569.867692] FS:  0000000000000000(0000) GS:ffff880407300000(0000) knlGS:0000000000000000
[348569.867701] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[348569.867709] CR2: ffffc90303ff73b5 CR3: 0000000346c0d004 CR4: 00000000003606e0


(gdb) list *vivid_fillbuff+0x1e9b
0x1936b is in vivid_fillbuff (drivers/media/platform/vivid/vivid-kthread-cap.c:495).
490					ms % 1000,
491					buf->vb.sequence,
492					(dev->field_cap == V4L2_FIELD_ALTERNATE) ?
493						(buf->vb.field == V4L2_FIELD_TOP ?
494						 " top" : " bottom") : "");
495			tpg_gen_text(tpg, basep, line++ * line_height, 16, str);
496		}
497		if (dev->osd_mode == 0) {
498			snprintf(str, sizeof(str), " %dx%d, input %d ",
499					dev->src_rect.width, dev->src_rect.height, dev->input);



Thanks,
Mauro

  reply	other threads:[~2018-10-09  1:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-03  7:06 [PATCH] media: vivid: Support 480p for webcam capture Keiichi Watanabe
2018-10-03  7:08 ` Keiichi Watanabe
2018-10-03 11:16   ` Kieran Bingham
2018-10-05  9:18   ` Hans Verkuil
2018-10-06 10:29     ` Keiichi Watanabe
2018-10-08  8:35       ` Kieran Bingham
2018-10-08  9:00         ` Hans Verkuil
2018-10-08  9:47           ` Kieran Bingham
2018-10-03 11:14 ` Kieran Bingham
2018-10-08 17:03   ` Mauro Carvalho Chehab
2018-10-08 17:53     ` Hans Verkuil
2018-10-08 18:23       ` Mauro Carvalho Chehab [this message]
2018-10-08 18:28         ` Mauro Carvalho Chehab
2018-10-08 18:31         ` Hans Verkuil
2018-10-08 19:00           ` Mauro Carvalho Chehab
2018-10-09  6:01             ` Keiichi Watanabe
2018-10-09  6:33               ` Hans Verkuil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181008152348.7ef6d77e@coco.lan \
    --to=mchehab+samsung@kernel.org \
    --cc=hverkuil@xs4all.nl \
    --cc=jcliang@chromium.org \
    --cc=keiichiw@chromium.org \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@kernel.org \
    --cc=shik@chromium.org \
    --cc=tfiga@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox