V4L/DVB: pvrusb2: Enforce a 300msec stabilization interval during stream strart

Martin Dauskardt <martin.dauskardt@gmx.de> has determined that the encoder has a much better chance of starting cleanly if we deliberately hold off starting it util the video digitizer has had a chance to run for at least 300msec first. These changes implement an enforced 300msec wait in the state machine that orchestrates streaming start / stop. Signed-off-by: Mike Isely <isely@pobox.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

V4L/DVB: pvrusb2: Enforce a 300msec stabilization interval during stream strart
Martin Dauskardt <martin.dauskardt@gmx.de> has determined that the encoder has a much better chance of starting cleanly if we deliberately hold off starting it util the video digitizer has had a chance to run for at least 300msec first. These changes implement an enforced 300msec wait in the state machine that orchestrates streaming start / stop. Signed-off-by: Mike Isely <isely@pobox.com> Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
Mike Isely · Mauro Carvalho Chehab
1 parent 7cae112ebe
Showing 3 changed files with 49 additions and 5 deletions Side-by-side Diff
drivers/media/video/pvrusb2/pvrusb2-hdw-internal.h
drivers/media/video/pvrusb2/pvrusb2-hdw.c
drivers/media/video/pvrusb2/pvrusb2-hdw.h
@@ -233,8 +233,9 @@
 	int state_encoder_waitok;     /* Encoder pre-wait done */
 	int state_encoder_runok;      /* Encoder has run for >= .25 sec */
 	int state_decoder_run;        /* Decoder is running */
+	int state_decoder_ready;      /* Decoder is stabilized & streamable */
 	int state_usbstream_run;      /* FX2 is streaming */
-	int state_decoder_quiescent;  /* Decoder idle for > 50msec */
+	int state_decoder_quiescent;  /* Decoder idle for minimal interval */
 	int state_pipeline_config;    /* Pipeline is configured */
 	int state_pipeline_req;       /* Somebody wants to stream */
 	int state_pipeline_pause;     /* Pipeline must be paused */
  
@@ -255,8 +256,15 @@
 	void (*state_func)(void *);
 	void *state_data;
  
-	/* Timer for measuring decoder settling time */
+	/* Timer for measuring required decoder settling time before we're
+	   allowed to fire it up again. */
 	struct timer_list quiescent_timer;
+
+	/* Timer for measuring decoder stabilization time, which is the
+	   amount of time we need to let the decoder run before we can
+	   trust its output (otherwise the encoder might see garbage and
+	   then fail to start correctly). */
+	struct timer_list decoder_stabilization_timer;
  
 	/* Timer for measuring encoder pre-wait time */
 	struct timer_list encoder_wait_timer;
@@ -48,6 +48,10 @@
    before we are allowed to start it running. */
 #define TIME_MSEC_DECODER_WAIT 50
  
+/* This defines a minimum interval that the decoder must be allowed to run
+   before we can safely begin using its streaming output. */
+#define TIME_MSEC_DECODER_STABILIZATION_WAIT 300
+
 /* This defines a minimum interval that the encoder must remain quiet
    before we are allowed to configure it.  I had this originally set to
    50msec, but Martin Dauskardt <martin.dauskardt@gmx.de> reports that
@@ -334,6 +338,7 @@
 static void pvr2_hdw_internal_find_stdenum(struct pvr2_hdw *hdw);
 static void pvr2_hdw_internal_set_std_avail(struct pvr2_hdw *hdw);
 static void pvr2_hdw_quiescent_timeout(unsigned long);
+static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
 static void pvr2_hdw_encoder_wait_timeout(unsigned long);
 static void pvr2_hdw_encoder_run_timeout(unsigned long);
 static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
@@ -2462,6 +2467,11 @@
 	hdw->quiescent_timer.data = (unsigned long)hdw;
 	hdw->quiescent_timer.function = pvr2_hdw_quiescent_timeout;
  
+	init_timer(&hdw->decoder_stabilization_timer);
+	hdw->decoder_stabilization_timer.data = (unsigned long)hdw;
+	hdw->decoder_stabilization_timer.function =
+		pvr2_hdw_decoder_stabilization_timeout;
+
 	init_timer(&hdw->encoder_wait_timer);
 	hdw->encoder_wait_timer.data = (unsigned long)hdw;
 	hdw->encoder_wait_timer.function = pvr2_hdw_encoder_wait_timeout;
@@ -2675,6 +2685,7 @@
  fail:
 	if (hdw) {
 		del_timer_sync(&hdw->quiescent_timer);
+		del_timer_sync(&hdw->decoder_stabilization_timer);
 		del_timer_sync(&hdw->encoder_run_timer);
 		del_timer_sync(&hdw->encoder_wait_timer);
 		if (hdw->workqueue) {
@@ -2742,6 +2753,7 @@
 		hdw->workqueue = NULL;
 	}
 	del_timer_sync(&hdw->quiescent_timer);
+	del_timer_sync(&hdw->decoder_stabilization_timer);
 	del_timer_sync(&hdw->encoder_run_timer);
 	del_timer_sync(&hdw->encoder_wait_timer);
 	if (hdw->fw_buffer) {
@@ -4453,7 +4465,7 @@
  
 	switch (hdw->pathway_state) {
 	case PVR2_PATHWAY_ANALOG:
-		if (hdw->state_decoder_run) {
+		if (hdw->state_decoder_run && hdw->state_decoder_ready) {
 			/* In analog mode, if the decoder is running, then
 			   run the encoder. */
 			return !0;
@@ -4520,6 +4532,17 @@
 }
  
  
+/* Timeout function for decoder stabilization timer. */
+static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
+{
+	struct pvr2_hdw *hdw = (struct pvr2_hdw *)data;
+	hdw->state_decoder_ready = !0;
+	trace_stbit("state_decoder_ready", hdw->state_decoder_ready);
+	hdw->state_stale = !0;
+	queue_work(hdw->workqueue, &hdw->workpoll);
+}
+
+
 /* Timeout function for encoder wait timer. */
 static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
 {
  
@@ -4558,8 +4581,13 @@
 		}
 		hdw->state_decoder_quiescent = 0;
 		hdw->state_decoder_run = 0;
-		/* paranoia - solve race if timer just completed */
+		/* paranoia - solve race if timer(s) just completed */
 		del_timer_sync(&hdw->quiescent_timer);
+		/* Kill the stabilization timer, in case we're killing the
+		   encoder before the previous stabilization interval has
+		   been properly timed. */
+		del_timer_sync(&hdw->decoder_stabilization_timer);
+		hdw->state_decoder_ready = 0;
 	} else {
 		if (!hdw->state_decoder_quiescent) {
 			if (!timer_pending(&hdw->quiescent_timer)) {
  
  
@@ -4597,10 +4625,16 @@
 		if (hdw->flag_decoder_missed) return 0;
 		if (pvr2_decoder_enable(hdw,!0) < 0) return 0;
 		hdw->state_decoder_quiescent = 0;
+		hdw->state_decoder_ready = 0;
 		hdw->state_decoder_run = !0;
+		hdw->decoder_stabilization_timer.expires =
+			jiffies +
+			(HZ * TIME_MSEC_DECODER_STABILIZATION_WAIT / 1000);
+		add_timer(&hdw->decoder_stabilization_timer);
 	}
 	trace_stbit("state_decoder_quiescent",hdw->state_decoder_quiescent);
 	trace_stbit("state_decoder_run",hdw->state_decoder_run);
+	trace_stbit("state_decoder_ready", hdw->state_decoder_ready);
 	return !0;
 }
  
@@ -4798,7 +4832,8 @@
 			buf,acnt,
 			"worker:%s%s%s%s%s%s%s",
 			(hdw->state_decoder_run ?
-			 " <decode:run>" :
+			 (hdw->state_decoder_ready ?
+			  "<decode:run>" : " <decode:start>") :
 			 (hdw->state_decoder_quiescent ?
 			  "" : " <decode:stop>")),
 			(hdw->state_decoder_quiescent ?
@@ -306,6 +306,7 @@
 	int state_encoder_ok;
 	int state_encoder_run;
 	int state_decoder_run;
+	int state_decoder_ready;
 	int state_usbstream_run;
 	int state_decoder_quiescent;
 	int state_pipeline_config;
...	...	@@ -233,8 +233,9 @@
233	233	int state_encoder_waitok; /* Encoder pre-wait done */
234	234	int state_encoder_runok; /* Encoder has run for >= .25 sec */
235	235	int state_decoder_run; /* Decoder is running */
	236	+ int state_decoder_ready; /* Decoder is stabilized & streamable */
236	237	int state_usbstream_run; /* FX2 is streaming */
237		- int state_decoder_quiescent; /* Decoder idle for > 50msec */
	238	+ int state_decoder_quiescent; /* Decoder idle for minimal interval */
238	239	int state_pipeline_config; /* Pipeline is configured */
239	240	int state_pipeline_req; /* Somebody wants to stream */
240	241	int state_pipeline_pause; /* Pipeline must be paused */
241	242
...	...	@@ -255,8 +256,15 @@
255	256	void (state_func)(void );
256	257	void *state_data;
257	258
258		- /* Timer for measuring decoder settling time */
	259	+ /* Timer for measuring required decoder settling time before we're
	260	+ allowed to fire it up again. */
259	261	struct timer_list quiescent_timer;
	262	+
	263	+ /* Timer for measuring decoder stabilization time, which is the
	264	+ amount of time we need to let the decoder run before we can
	265	+ trust its output (otherwise the encoder might see garbage and
	266	+ then fail to start correctly). */
	267	+ struct timer_list decoder_stabilization_timer;
260	268
261	269	/* Timer for measuring encoder pre-wait time */
262	270	struct timer_list encoder_wait_timer;
...	...	@@ -48,6 +48,10 @@
48	48	before we are allowed to start it running. */
49	49	#define TIME_MSEC_DECODER_WAIT 50
50	50
	51	+/* This defines a minimum interval that the decoder must be allowed to run
	52	+ before we can safely begin using its streaming output. */
	53	+#define TIME_MSEC_DECODER_STABILIZATION_WAIT 300
	54	+
51	55	/* This defines a minimum interval that the encoder must remain quiet
52	56	before we are allowed to configure it. I had this originally set to
53	57	50msec, but Martin Dauskardt <martin.dauskardt@gmx.de> reports that
...	...	@@ -334,6 +338,7 @@
334	338	static void pvr2_hdw_internal_find_stdenum(struct pvr2_hdw *hdw);
335	339	static void pvr2_hdw_internal_set_std_avail(struct pvr2_hdw *hdw);
336	340	static void pvr2_hdw_quiescent_timeout(unsigned long);
	341	+static void pvr2_hdw_decoder_stabilization_timeout(unsigned long);
337	342	static void pvr2_hdw_encoder_wait_timeout(unsigned long);
338	343	static void pvr2_hdw_encoder_run_timeout(unsigned long);
339	344	static int pvr2_issue_simple_cmd(struct pvr2_hdw *,u32);
...	...	@@ -2462,6 +2467,11 @@
2462	2467	hdw->quiescent_timer.data = (unsigned long)hdw;
2463	2468	hdw->quiescent_timer.function = pvr2_hdw_quiescent_timeout;
2464	2469
	2470	+ init_timer(&hdw->decoder_stabilization_timer);
	2471	+ hdw->decoder_stabilization_timer.data = (unsigned long)hdw;
	2472	+ hdw->decoder_stabilization_timer.function =
	2473	+ pvr2_hdw_decoder_stabilization_timeout;
	2474	+
2465	2475	init_timer(&hdw->encoder_wait_timer);
2466	2476	hdw->encoder_wait_timer.data = (unsigned long)hdw;
2467	2477	hdw->encoder_wait_timer.function = pvr2_hdw_encoder_wait_timeout;
...	...	@@ -2675,6 +2685,7 @@
2675	2685	fail:
2676	2686	if (hdw) {
2677	2687	del_timer_sync(&hdw->quiescent_timer);
	2688	+ del_timer_sync(&hdw->decoder_stabilization_timer);
2678	2689	del_timer_sync(&hdw->encoder_run_timer);
2679	2690	del_timer_sync(&hdw->encoder_wait_timer);
2680	2691	if (hdw->workqueue) {
...	...	@@ -2742,6 +2753,7 @@
2742	2753	hdw->workqueue = NULL;
2743	2754	}
2744	2755	del_timer_sync(&hdw->quiescent_timer);
	2756	+ del_timer_sync(&hdw->decoder_stabilization_timer);
2745	2757	del_timer_sync(&hdw->encoder_run_timer);
2746	2758	del_timer_sync(&hdw->encoder_wait_timer);
2747	2759	if (hdw->fw_buffer) {
...	...	@@ -4453,7 +4465,7 @@
4453	4465
4454	4466	switch (hdw->pathway_state) {
4455	4467	case PVR2_PATHWAY_ANALOG:
4456		- if (hdw->state_decoder_run) {
	4468	+ if (hdw->state_decoder_run && hdw->state_decoder_ready) {
4457	4469	/* In analog mode, if the decoder is running, then
4458	4470	run the encoder. */
4459	4471	return !0;
...	...	@@ -4520,6 +4532,17 @@
4520	4532	}
4521	4533
4522	4534
	4535	+/* Timeout function for decoder stabilization timer. */
	4536	+static void pvr2_hdw_decoder_stabilization_timeout(unsigned long data)
	4537	+{
	4538	+ struct pvr2_hdw hdw = (struct pvr2_hdw )data;
	4539	+ hdw->state_decoder_ready = !0;
	4540	+ trace_stbit("state_decoder_ready", hdw->state_decoder_ready);
	4541	+ hdw->state_stale = !0;
	4542	+ queue_work(hdw->workqueue, &hdw->workpoll);
	4543	+}
	4544	+
	4545	+
4523	4546	/* Timeout function for encoder wait timer. */
4524	4547	static void pvr2_hdw_encoder_wait_timeout(unsigned long data)
4525	4548	{
4526	4549
...	...	@@ -4558,8 +4581,13 @@
4558	4581	}
4559	4582	hdw->state_decoder_quiescent = 0;
4560	4583	hdw->state_decoder_run = 0;
4561		- /* paranoia - solve race if timer just completed */
	4584	+ /* paranoia - solve race if timer(s) just completed */
4562	4585	del_timer_sync(&hdw->quiescent_timer);
	4586	+ /* Kill the stabilization timer, in case we're killing the
	4587	+ encoder before the previous stabilization interval has
	4588	+ been properly timed. */
	4589	+ del_timer_sync(&hdw->decoder_stabilization_timer);
	4590	+ hdw->state_decoder_ready = 0;
4563	4591	} else {
4564	4592	if (!hdw->state_decoder_quiescent) {
4565	4593	if (!timer_pending(&hdw->quiescent_timer)) {
4566	4594
4567	4595
...	...	@@ -4597,10 +4625,16 @@
4597	4625	if (hdw->flag_decoder_missed) return 0;
4598	4626	if (pvr2_decoder_enable(hdw,!0) < 0) return 0;
4599	4627	hdw->state_decoder_quiescent = 0;
	4628	+ hdw->state_decoder_ready = 0;
4600	4629	hdw->state_decoder_run = !0;
	4630	+ hdw->decoder_stabilization_timer.expires =
	4631	+ jiffies +
	4632	+ (HZ * TIME_MSEC_DECODER_STABILIZATION_WAIT / 1000);
	4633	+ add_timer(&hdw->decoder_stabilization_timer);
4601	4634	}
4602	4635	trace_stbit("state_decoder_quiescent",hdw->state_decoder_quiescent);
4603	4636	trace_stbit("state_decoder_run",hdw->state_decoder_run);
	4637	+ trace_stbit("state_decoder_ready", hdw->state_decoder_ready);
4604	4638	return !0;
4605	4639	}
4606	4640
...	...	@@ -4798,7 +4832,8 @@
4798	4832	buf,acnt,
4799	4833	"worker:%s%s%s%s%s%s%s",
4800	4834	(hdw->state_decoder_run ?
4801		- " <decode:run>" :
	4835	+ (hdw->state_decoder_ready ?
	4836	+ "<decode:run>" : " <decode:start>") :
4802	4837	(hdw->state_decoder_quiescent ?
4803	4838	"" : " <decode:stop>")),
4804	4839	(hdw->state_decoder_quiescent ?
...	...	@@ -306,6 +306,7 @@
306	306	int state_encoder_ok;
307	307	int state_encoder_run;
308	308	int state_decoder_run;
	309	+ int state_decoder_ready;
309	310	int state_usbstream_run;
310	311	int state_decoder_quiescent;
311	312	int state_pipeline_config;