diff mbox

DVB-APPS: azap gets -p argument

Message ID 20110315123258.GA6570@aniel (mailing list archive)
State Not Applicable
Headers show

Commit Message

Janne Grunau March 15, 2011, 12:32 p.m. UTC
None

Comments

Janne Grunau April 4, 2011, 11:05 a.m. UTC | #1
On Tue, Mar 15, 2011 at 02:50:05PM +0100, Oliver Endriss wrote:
> On Tuesday 15 March 2011 13:32:58 Janne Grunau wrote:
> > On Tue, Mar 15, 2011 at 01:23:40PM +0100, Christian Ulrich wrote:
> > > Hi, thank you for your feedback.
> > > 
> > > Indeed, I never used -r alone, but only with -p.
> > > So with your patch, [acst]zap -r will be the same as -rp. That looks good to me.
> > 
> > well, azap not yet. iirc I implemented -p for azap but it was never
> > applied since nobody tested it. see attached patch for [cst]zap
> 
> NAK.

I think we had the same discussion when I submitted -p for czap and
tzap.

> The PAT/PMT from the stream does not describe the dvr stream correctly.
> 
> The dvr device provides *some* PIDs of the transponder, while the
> PAT/PMT reference *all* programs of the transponder.

True, the PAT references some PMT pids which won't be included. All pids
from the desired program should be included. A transport stream without
PAT/PMT is as invalid as the stream with incorrect PAT/PMT/missing pids
but the second is easier to handle for player software than the first.

> For correct results the PAT/PMT has to be re-created.

That's not possible from ?zap and I hope you don't suggest we add
PMT/PAT rewriting routines to kernel software demuxer.

> The separate -p option seems acceptable - as a debug feature.

-r is as much a debug feature as -p. the output is invalid too

Janne
--
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
Oliver Endriss April 6, 2011, 6:39 a.m. UTC | #2
On Monday 04 April 2011 13:05:19 Janne Grunau wrote:
> On Tue, Mar 15, 2011 at 02:50:05PM +0100, Oliver Endriss wrote:
> > On Tuesday 15 March 2011 13:32:58 Janne Grunau wrote:
> > > On Tue, Mar 15, 2011 at 01:23:40PM +0100, Christian Ulrich wrote:
> > > > Hi, thank you for your feedback.
> > > > 
> > > > Indeed, I never used -r alone, but only with -p.
> > > > So with your patch, [acst]zap -r will be the same as -rp. That looks good to me.
> > > 
> > > well, azap not yet. iirc I implemented -p for azap but it was never
> > > applied since nobody tested it. see attached patch for [cst]zap
> > 
> > NAK.
> 
> I think we had the same discussion when I submitted -p for czap and
> tzap.

I don't care about an additional option, but the behaviour with option
'-r' should not change. There is no need to do so.

> > The PAT/PMT from the stream does not describe the dvr stream correctly.
> > 
> > The dvr device provides *some* PIDs of the transponder, while the
> > PAT/PMT reference *all* programs of the transponder.
> 
> True, the PAT references some PMT pids which won't be included. All pids
> from the desired program should be included. A transport stream without
> PAT/PMT is as invalid as the stream with incorrect PAT/PMT/missing pids
> but the second is easier to handle for player software than the first.

A sane player can handle a TS stream without PAT/PMT.
Iirc mplayer never had any problems.

> > For correct results the PAT/PMT has to be re-created.
> 
> That's not possible from ?zap and I hope you don't suggest we add
> PMT/PAT rewriting routines to kernel software demuxer.

No. ;-)

> > The separate -p option seems acceptable - as a debug feature.
> 
> -r is as much a debug feature as -p. the output is invalid too

With separate options, you have a choice. So implement a separate option
'-p', and everything is fine.

CU
Oliver
Janne Grunau April 6, 2011, 7:15 a.m. UTC | #3
On Wed, Apr 06, 2011 at 08:39:05AM +0200, Oliver Endriss wrote:
> On Monday 04 April 2011 13:05:19 Janne Grunau wrote:
> > On Tue, Mar 15, 2011 at 02:50:05PM +0100, Oliver Endriss wrote:
> > > The PAT/PMT from the stream does not describe the dvr stream correctly.
> > > 
> > > The dvr device provides *some* PIDs of the transponder, while the
> > > PAT/PMT reference *all* programs of the transponder.
> > 
> > True, the PAT references some PMT pids which won't be included. All pids
> > from the desired program should be included. A transport stream without
> > PAT/PMT is as invalid as the stream with incorrect PAT/PMT/missing pids
> > but the second is easier to handle for player software than the first.
> 
> A sane player can handle a TS stream without PAT/PMT.
> Iirc mplayer never had any problems.

mplayer with default options has only no problems as long as the video
codec is mpeg2 and possible mpeg 1 layer 2 audio. Try any H.264 stream
and see it fail. That was the reason why I want to change the behaviour
with -r in the first place. http://blog.fefe.de/?ts=b58fb6b1 (german
content) triggered it.

I don't care too much. Can someone please push Christian's original
patch adding -p to azap.

Janne
--
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
diff mbox

Patch

diff -r 4746d76ae4b6 util/szap/czap.c
--- a/util/szap/czap.c	Sat Mar 05 18:39:58 2011 +0100
+++ b/util/szap/czap.c	Tue Mar 15 13:32:29 2011 +0100
@@ -253,7 +253,7 @@ 
     "     -x        : exit after tuning\n"
     "     -H        : human readable output\n"
     "     -r        : set up /dev/dvb/adapterX/dvr0 for TS recording\n"
-    "     -p        : add pat and pmt to TS recording (implies -r)\n"
+    "     -p        : obsolete (pat and pmt will always be included with -r)\n"
 ;
 
 int main(int argc, char **argv)
@@ -279,8 +279,11 @@ 
 		case 'd':
 			demux = strtoul(optarg, NULL, 0);
 			break;
+		case 'p':
+			printf("'-p' is obsolete. '-r' records PAT/PMT");
 		case 'r':
 			dvr = 1;
+			rec_psi = 1;
 			break;
 		case 'l':
 			list_channels = 1;
@@ -288,9 +291,6 @@ 
 		case 'n':
 			chan_no = strtoul(optarg, NULL, 0);
 			break;
-		case 'p':
-			rec_psi = 1;
-			break;
 		case 'x':
 			exit_after_tuning = 1;
 			break;
diff -r 4746d76ae4b6 util/szap/szap.c
--- a/util/szap/szap.c	Sat Mar 05 18:39:58 2011 +0100
+++ b/util/szap/szap.c	Tue Mar 15 13:32:29 2011 +0100
@@ -547,8 +547,11 @@ 
 	 case 'q':
 	    list_channels = 1;
 	    break;
+	 case 'p':
+	    printf("'-p' is obsolete. '-r' records PAT/PMT");
 	 case 'r':
 	    dvr = 1;
+	    rec_psi = 1;
 	    break;
 	 case 'n':
 	    chan_no = strtoul(optarg, NULL, 0);
@@ -559,9 +562,6 @@ 
 	 case 'f':
 	    frontend = strtoul(optarg, NULL, 0);
 	    break;
-	 case 'p':
-	    rec_psi = 1;
-	    break;
 	 case 'd':
 	    demux = strtoul(optarg, NULL, 0);
 	    break;
diff -r 4746d76ae4b6 util/szap/tzap.c
--- a/util/szap/tzap.c	Sat Mar 05 18:39:58 2011 +0100
+++ b/util/szap/tzap.c	Tue Mar 15 13:32:29 2011 +0100
@@ -485,7 +485,7 @@ 
     "     -c file   : read channels list from 'file'\n"
     "     -x        : exit after tuning\n"
     "     -r        : set up /dev/dvb/adapterX/dvr0 for TS recording\n"
-    "     -p        : add pat and pmt to TS recording (implies -r)\n"
+    "     -p        : obsolete (pat and pmt will always be included with -r)\n"
     "     -s        : only print summary\n"
     "     -S        : run silently (no output)\n"
     "     -H        : human readable output\n"
@@ -529,10 +529,10 @@ 
 			filename = strdup(optarg);
 			record=1;
 			/* fall through */
+		case 'p':
+			printf("'-p' is obsolete. '-r' records PAT/PMT");
 		case 'r':
 			dvr = 1;
-			break;
-		case 'p':
 			rec_psi = 1;
 			break;
 		case 'x':