dvbv5-tzap with pctv 290e/292e needs EAGAIN for pat/pmt to work when recording.

dvbv5-tzap with pctv 290e/292e needs EAGAIN for pat/pmt to work when recording.

[Andy Furniss]

Running kernel 3.18.14 with git master v4l-utils and a pctv290e + a 292e.

If I try to record with dvbv5-zap and include the "p" option to get
pat/pmt I get -

read_sections: read error: Resource temporarily unavailable
couldn't find pmt-pid for sid 10bf

Doing this this fixes it for me (obviously not meant to be a a proper 
patch).

diff --git a/lib/libdvbv5/dvb-demux.c b/lib/libdvbv5/dvb-demux.c
index 30d4eda..b520948 100644
--- a/lib/libdvbv5/dvb-demux.c
+++ b/lib/libdvbv5/dvb-demux.c
@@ -151,8 +151,10 @@ int dvb_get_pmt_pid(int patfd, int sid)
                 if (((count = read(patfd, buf, sizeof(buft))) < 0) && 
errno == EOVERFLOW)
                 count = read(patfd, buf, sizeof(buft));
                 if (count < 0) {
-               perror("read_sections: read error");
-               return -1;
+                       if (errno == EAGAIN) /*ADF*/
+                               continue;
+                       perror("read_sections: read error");
+                       return -1;
                 }

                 section_length = ((buf[1] & 0x0f) << 8) | buf[2];

Re: dvbv5-tzap with pctv 290e/292e needs EAGAIN for pat/pmt to work when recording.

[Mauro Carvalho Chehab]

Em Tue, 02 Jun 2015 23:25:31 +0100
Andy Furniss <adf.lists@gmail.com> escreveu:

> Running kernel 3.18.14 with git master v4l-utils and a pctv290e + a 292e.
> 
> If I try to record with dvbv5-zap and include the "p" option to get
> pat/pmt I get -
> 
> read_sections: read error: Resource temporarily unavailable
> couldn't find pmt-pid for sid 10bf
> 
> Doing this this fixes it for me (obviously not meant to be a a proper 
> patch).

You forgot to send your Signed-off-by on this patch ;)

Anyway, there are other places where EAGAIN may happen. So, the best
is to fix it globally.

Just applied a fix for it:
	http://git.linuxtv.org/cgit.cgi/v4l-utils.git/commit/?id=c7c9af17163f282a147ea76f1a3c0e9a0a86e7fa

It will retry up to 10 times. This should very likely be enough if the
driver doesn't have any bug.

Please let me know if this fixes the issue.

Regards,
Mauro

> 
> diff --git a/lib/libdvbv5/dvb-demux.c b/lib/libdvbv5/dvb-demux.c
> index 30d4eda..b520948 100644
> --- a/lib/libdvbv5/dvb-demux.c
> +++ b/lib/libdvbv5/dvb-demux.c
> @@ -151,8 +151,10 @@ int dvb_get_pmt_pid(int patfd, int sid)
>                  if (((count = read(patfd, buf, sizeof(buft))) < 0) && 
> errno == EOVERFLOW)
>                  count = read(patfd, buf, sizeof(buft));
>                  if (count < 0) {
> -               perror("read_sections: read error");
> -               return -1;
> +                       if (errno == EAGAIN) /*ADF*/
> +                               continue;
> +                       perror("read_sections: read error");
> +                       return -1;
>                  }
> 
>                  section_length = ((buf[1] & 0x0f) << 8) | buf[2];
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: dvbv5-tzap with pctv 290e/292e needs EAGAIN for pat/pmt to work when recording.

[Andy Furniss]

Mauro Carvalho Chehab wrote:

> Just applied a fix for it:
> 	http://git.linuxtv.org/cgit.cgi/v4l-utils.git/commit/?id=c7c9af17163f282a147ea76f1a3c0e9a0a86e7fa
>
> It will retry up to 10 times. This should very likely be enough if the
> driver doesn't have any bug.
>
> Please let me know if this fixes the issue.

No, it doesn't, so I reverted the above and added back my hack + a 
counter as below and it seems to be retrying > a million times.

Tested both 290e and 292e.

I currently have a post on linux-usb because I am having some dvb packet 
loss issues unless I spin a cpu - don't know if that's relevant or not 
but thought I should mention it. The loss is relatively low level but 
enough to be annoying, spinning a cpu doesn't change the eagain count.

results with below patch recording =

asr[scans-dvb]$ dvbv5-zap -a 1  -pro ~/test1.ts -t 10 -c 
dvb_channel-290.conf "BBC TWO"
using demux '/dev/dvb/adapter1/demux0'
reading channels from file 'dvb_channel-290.conf'
service has pid type 05:  7270 250
tuning to 745833000 Hz

EAGAIN count = 1468127

video pid 201
   dvb_set_pesfilter 201
audio pid 202
   dvb_set_pesfilter 202
Lock   (0x1f) Signal= 100.00% C/N= 0.20% UCB= 132 postBER= 0
Lock   (0x1f) Signal= 100.00% C/N= 0.21% UCB= 138 postBER= 0
Record to file '/home/andy/test1.ts' started
copied 4581936 bytes (447 Kbytes/sec)
Lock   (0x1f) Signal= 100.00% C/N= 0.43% UCB= 283 postBER= 0

asr[scans-dvb]$ dvbv5-zap -a 0  -pro ~/test2.ts -t 10 -c 
dvb_channel-290.conf "BBC TWO"
using demux '/dev/dvb/adapter0/demux0'
reading channels from file 'dvb_channel-290.conf'
service has pid type 05:  7270 250
tuning to 745833000 Hz

EAGAIN count = 1285533

video pid 201
   dvb_set_pesfilter 201
audio pid 202
   dvb_set_pesfilter 202
Lock   (0x1f) C/N= 16.75dB
Lock   (0x1f) C/N= 17.00dB
Record to file '/home/andy/test2.ts' started
copied 3780116 bytes (369 Kbytes/sec)
Lock   (0x1f) C/N= 34.00dB



diff --git a/lib/libdvbv5/dvb-demux.c b/lib/libdvbv5/dvb-demux.c
index 30d4eda..f435078 100644
--- a/lib/libdvbv5/dvb-demux.c
+++ b/lib/libdvbv5/dvb-demux.c
@@ -130,6 +130,7 @@ int dvb_get_pmt_pid(int patfd, int sid)
         int count;
         int pmt_pid = 0;
         int patread = 0;
+        int eacount = 0;
         int section_length;
         unsigned char buft[4096];
         unsigned char *buf = buft;
@@ -151,10 +152,16 @@ int dvb_get_pmt_pid(int patfd, int sid)
                 if (((count = read(patfd, buf, sizeof(buft))) < 0) && 
errno == EOVERFLOW)
                 count = read(patfd, buf, sizeof(buft));
                 if (count < 0) {
-               perror("read_sections: read error");
-               return -1;
+                       if (errno == EAGAIN){ /*ADF*/
+                               eacount++;
+                               continue;
+                       }
+                       perror("read_sections: read error");
+                       return -1;
                 }

+                fprintf(stderr, "EAGAIN count = %d\n", eacount);
+
                 section_length = ((buf[1] & 0x0f) << 8) | buf[2];
                 if (count != section_length + 3)
                 continue;

Re: dvbv5-tzap with pctv 290e/292e needs EAGAIN for pat/pmt to work when recording.

[Mauro Carvalho Chehab]

Em Wed, 10 Jun 2015 18:27:30 +0100
Andy Furniss <adf.lists@gmail.com> escreveu:

> Mauro Carvalho Chehab wrote:
> 
> > Just applied a fix for it:
> > 	http://git.linuxtv.org/cgit.cgi/v4l-utils.git/commit/?id=c7c9af17163f282a147ea76f1a3c0e9a0a86e7fa
> >
> > It will retry up to 10 times. This should very likely be enough if the
> > driver doesn't have any bug.
> >
> > Please let me know if this fixes the issue.
> 
> No, it doesn't, so I reverted the above and added back my hack + a 
> counter as below and it seems to be retrying > a million times.

Hmm.... that's likely a bug at the demod driver. It doesn't make much
sense to keep a mutex hold for that long. 

Anyway, I modified the patch to use a timeout of 1 second, instead of
trying 10 times. It is still a hack, as IMHO this is a driver bug,
but it should produce a better result.

Please check if the patch below works for you.

You may change the MAX_TIME there if 1 second is not enough.

It could be interesting if you add a printf with the difference
between start and end time, for us to have an idea about how
much time the driver is kept on such unreliable state.

Thanks!
Mauro


[PATCH] libdvbv5: use a timeout for ioctl

Some frontends don't play nice: they return -EAGAIN if the
device doesn't lock. That actually means that it may take
some time for some ioctl's to succeed. On experimental tests,
the loop may happen ~2 million times!

Well, better to waste power on a loop than to fail. So, let's
change the code that detects EAGAIN by a loop that waits up
to 1 second.

This is not the right thing to do, but the Kernel drivers
require fixes. We can do it only for newer versions of the
Kernel.

Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>

diff --git a/lib/libdvbv5/dvb-demux.c b/lib/libdvbv5/dvb-demux.c
index 867d7b9dddde..af124ae3a7cc 100644
--- a/lib/libdvbv5/dvb-demux.c
+++ b/lib/libdvbv5/dvb-demux.c
@@ -30,6 +30,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <stdio.h>
+#include <time.h>
 #include <errno.h>
 
 #include <sys/ioctl.h>
@@ -40,12 +41,25 @@
 
 #include <libdvbv5/dvb-demux.h>
 
+#define MAX_TIME		10	/* 1.0 seconds */
+
 #define xioctl(fh, request, arg...) ({					\
-	int __rc, __retry;						\
+	int __rc;							\
+	struct timespec __start, __end;					\
 									\
-	for (__retry = 0; __retry < 10; __retry++) {			\
+	clock_gettime(CLOCK_MONOTONIC, &__start);			\
+	do {								\
 		__rc = ioctl(fh, request, ##arg);			\
-	} while (__rc == -1 && ((errno == EINTR) || (errno == EAGAIN)));\
+		if (__rc != -1)						\
+			break;						\
+		if (!((errno == EINTR) | (errno == EAGAIN)))		\
+			break;						\
+		clock_gettime(CLOCK_MONOTONIC, &__end);			\
+		if (__end.tv_sec * 10 + __end.tv_nsec / 100000000 >	\
+		    __start.tv_sec * 10 + __start.tv_nsec / 100000000 +	\
+		    MAX_TIME)						\
+			break;						\
+	} while (1);							\
 									\
 	__rc;								\
 })
diff --git a/lib/libdvbv5/dvb-fe.c b/lib/libdvbv5/dvb-fe.c
index 48b09cd9ceaa..8607401841f2 100644
--- a/lib/libdvbv5/dvb-fe.c
+++ b/lib/libdvbv5/dvb-fe.c
@@ -26,6 +26,7 @@
 #include <inttypes.h>
 #include <math.h>
 #include <stddef.h>
+#include <time.h>
 #include <unistd.h>
 
 #include <config.h>
@@ -43,12 +44,25 @@ static int libdvbv5_initialized = 0;
 
 # define N_(string) string
 
+#define MAX_TIME		10	/* 1.0 seconds */
+
 #define xioctl(fh, request, arg...) ({					\
-	int __rc, __retry;						\
+	int __rc;							\
+	struct timespec __start, __end;					\
 									\
-	for (__retry = 0; __retry < 10; __retry++) {			\
+	clock_gettime(CLOCK_MONOTONIC, &__start);			\
+	do {								\
 		__rc = ioctl(fh, request, ##arg);			\
-	} while (__rc == -1 && ((errno == EINTR) || (errno == EAGAIN)));\
+		if (__rc != -1)						\
+			break;						\
+		if (!((errno == EINTR) | (errno == EAGAIN)))		\
+			break;						\
+		clock_gettime(CLOCK_MONOTONIC, &__end);			\
+		if (__end.tv_sec * 10 + __end.tv_nsec / 100000000 >	\
+		    __start.tv_sec * 10 + __start.tv_nsec / 100000000 +	\
+		    MAX_TIME)						\
+			break;						\
+	} while (1);							\
 									\
 	__rc;								\
 })

Re: dvbv5-tzap with pctv 290e/292e needs EAGAIN for pat/pmt to work when recording.

[Mauro Carvalho Chehab]

Em Wed, 10 Jun 2015 15:50:47 -0300
Mauro Carvalho Chehab <mchehab@osg.samsung.com> escreveu:

> Em Wed, 10 Jun 2015 18:27:30 +0100
> Andy Furniss <adf.lists@gmail.com> escreveu:
> 
> > Mauro Carvalho Chehab wrote:
> > 
> > > Just applied a fix for it:
> > > 	http://git.linuxtv.org/cgit.cgi/v4l-utils.git/commit/?id=c7c9af17163f282a147ea76f1a3c0e9a0a86e7fa
> > >
> > > It will retry up to 10 times. This should very likely be enough if the
> > > driver doesn't have any bug.
> > >
> > > Please let me know if this fixes the issue.
> > 
> > No, it doesn't, so I reverted the above and added back my hack + a 
> > counter as below and it seems to be retrying > a million times.
> 
> Hmm.... that's likely a bug at the demod driver. It doesn't make much
> sense to keep a mutex hold for that long. 
> 
> Anyway, I modified the patch to use a timeout of 1 second, instead of
> trying 10 times. It is still a hack, as IMHO this is a driver bug,
> but it should produce a better result.
> 
> Please check if the patch below works for you.
> 
> You may change the MAX_TIME there if 1 second is not enough.
> 
> It could be interesting if you add a printf with the difference
> between start and end time, for us to have an idea about how
> much time the driver is kept on such unreliable state.

Actually, there was an error on that patch. I did some tests here
with a PCTV 292e. While I was not able to reproduce the issue
you're reporting, I forced some errors. The patch should be
working. The only question is if 1 second is enough or not.

So, please test.

PS.: the patch was already merged upstream.

Regards,
Mauro

Re: dvbv5-tzap with pctv 290e/292e needs EAGAIN for pat/pmt to work when recording.

[Andy Furniss]

Mauro Carvalho Chehab wrote:

> Actually, there was an error on that patch. I did some tests here
> with a PCTV 292e. While I was not able to reproduce the issue you're
> reporting, I forced some errors. The patch should be working. The
> only question is if 1 second is enough or not.
>
> So, please test.
>
> PS.: the patch was already merged upstream.

OK testing clean updated tree - it still instantly bails.

I am a bit confused how these patches would help - I am getting the
EAGAIN on the data read in dvb_get_pmt_pid not the ioctl.

Maybe this read should be delayed until lock is stable like the actual
recording seems to be?

Re: dvbv5-tzap with pctv 290e/292e needs EAGAIN for pat/pmt to work when recording.

[Andy Furniss]

Mauro Carvalho Chehab wrote:
> Em Wed, 10 Jun 2015 18:27:30 +0100
> Andy Furniss <adf.lists@gmail.com> escreveu:
>
>> Mauro Carvalho Chehab wrote:
>>
>>> Just applied a fix for it:
>>> 	http://git.linuxtv.org/cgit.cgi/v4l-utils.git/commit/?id=c7c9af17163f282a147ea76f1a3c0e9a0a86e7fa
>>>
>>> It will retry up to 10 times. This should very likely be enough if the
>>> driver doesn't have any bug.
>>>
>>> Please let me know if this fixes the issue.
>>
>> No, it doesn't, so I reverted the above and added back my hack + a
>> counter as below and it seems to be retrying > a million times.
>
> Hmm.... that's likely a bug at the demod driver. It doesn't make much
> sense to keep a mutex hold for that long.
>
> Anyway, I modified the patch to use a timeout of 1 second, instead of
> trying 10 times. It is still a hack, as IMHO this is a driver bug,
> but it should produce a better result.
>
> Please check if the patch below works for you.
>
> You may change the MAX_TIME there if 1 second is not enough.
>
> It could be interesting if you add a printf with the difference
> between start and end time, for us to have an idea about how
> much time the driver is kept on such unreliable state.
>
> Thanks!
> Mauro
>
>
> [PATCH] libdvbv5: use a timeout for ioctl
>
> Some frontends don't play nice: they return -EAGAIN if the
> device doesn't lock. That actually means that it may take
> some time for some ioctl's to succeed. On experimental tests,
> the loop may happen ~2 million times!
>
> Well, better to waste power on a loop than to fail. So, let's
> change the code that detects EAGAIN by a loop that waits up
> to 1 second.
>
> This is not the right thing to do, but the Kernel drivers
> require fixes. We can do it only for newer versions of the
> Kernel.

I can't get this to work - it instantly bails -

read_sections: read error: Resource temporarily unavailable
couldn't find pmt-pid for sid 10bf

will try more later/tomorrow.

I notice there are a couple of suspect bitwise | but changing to || 
didn't help.

>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
>
> diff --git a/lib/libdvbv5/dvb-demux.c b/lib/libdvbv5/dvb-demux.c
> index 867d7b9dddde..af124ae3a7cc 100644
> --- a/lib/libdvbv5/dvb-demux.c
> +++ b/lib/libdvbv5/dvb-demux.c
> @@ -30,6 +30,7 @@
>   #include <string.h>
>   #include <unistd.h>
>   #include <stdio.h>
> +#include <time.h>
>   #include <errno.h>
>
>   #include <sys/ioctl.h>
> @@ -40,12 +41,25 @@
>
>   #include <libdvbv5/dvb-demux.h>
>
> +#define MAX_TIME		10	/* 1.0 seconds */
> +
>   #define xioctl(fh, request, arg...) ({					\
> -	int __rc, __retry;						\
> +	int __rc;							\
> +	struct timespec __start, __end;					\
>   									\
> -	for (__retry = 0; __retry < 10; __retry++) {			\
> +	clock_gettime(CLOCK_MONOTONIC, &__start);			\
> +	do {								\
>   		__rc = ioctl(fh, request, ##arg);			\
> -	} while (__rc == -1 && ((errno == EINTR) || (errno == EAGAIN)));\
> +		if (__rc != -1)						\
> +			break;						\
> +		if (!((errno == EINTR) | (errno == EAGAIN)))		\
> +			break;						\
> +		clock_gettime(CLOCK_MONOTONIC, &__end);			\
> +		if (__end.tv_sec * 10 + __end.tv_nsec / 100000000 >	\
> +		    __start.tv_sec * 10 + __start.tv_nsec / 100000000 +	\
> +		    MAX_TIME)						\
> +			break;						\
> +	} while (1);							\
>   									\
>   	__rc;								\
>   })
> diff --git a/lib/libdvbv5/dvb-fe.c b/lib/libdvbv5/dvb-fe.c
> index 48b09cd9ceaa..8607401841f2 100644
> --- a/lib/libdvbv5/dvb-fe.c
> +++ b/lib/libdvbv5/dvb-fe.c
> @@ -26,6 +26,7 @@
>   #include <inttypes.h>
>   #include <math.h>
>   #include <stddef.h>
> +#include <time.h>
>   #include <unistd.h>
>
>   #include <config.h>
> @@ -43,12 +44,25 @@ static int libdvbv5_initialized = 0;
>
>   # define N_(string) string
>
> +#define MAX_TIME		10	/* 1.0 seconds */
> +
>   #define xioctl(fh, request, arg...) ({					\
> -	int __rc, __retry;						\
> +	int __rc;							\
> +	struct timespec __start, __end;					\
>   									\
> -	for (__retry = 0; __retry < 10; __retry++) {			\
> +	clock_gettime(CLOCK_MONOTONIC, &__start);			\
> +	do {								\
>   		__rc = ioctl(fh, request, ##arg);			\
> -	} while (__rc == -1 && ((errno == EINTR) || (errno == EAGAIN)));\
> +		if (__rc != -1)						\
> +			break;						\
> +		if (!((errno == EINTR) | (errno == EAGAIN)))		\
> +			break;						\
> +		clock_gettime(CLOCK_MONOTONIC, &__end);			\
> +		if (__end.tv_sec * 10 + __end.tv_nsec / 100000000 >	\
> +		    __start.tv_sec * 10 + __start.tv_nsec / 100000000 +	\
> +		    MAX_TIME)						\
> +			break;						\
> +	} while (1);							\
>   									\
>   	__rc;								\
>   })
>
>