Bug found in streamts

Snakehand
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Mittwoch 7. März 2007, 17:52

Bug found in streamts

Beitrag von Snakehand »

I think I have found a bug in streamts.c

At the end of the file these lines:

/* make sure to start with a ts header */
offset = sync_byte_offset(buf, TS_SIZE);

if (offset == -1)
continue;

packet_stdout(buf + offset, r - offset, NULL);


The demux should normally give data that keeps the ts block alligned, but if it looses allignment this piece of code won't fix the problem, but makes it much much worse. Instead of correcting allignement, it causes every 362 block to be truncated.

Also there is a inconsistency in the code. The previous loop should read IN_SIZE bytes, but if for any reason read gets called more than once, only the last (r) number of bytes will be sendt.

http://www.ridax.se/dreambox/ describes the problems that this causes on my DM 500C, but does not seem to have understood the GPL when he charges 10 euros for the fix.

As far as I am concerned some of these lines could just be deleted. If the stream gets broken, I trust VLC will do a much better job of syncing it in that streamts can do. Trying to correct lost sync is probably just more work than it is worth. It might make sense in UPD traffic, but probably not in TCP...

I tried to fix this myself, but libpng won't link when I do a make all...

Can someone with CVS write access please try to fix :-)

( Please answer in German if that is more convenient )
new.life
Erleuchteter
Erleuchteter
Beiträge: 797
Registriert: Sonntag 19. Februar 2006, 01:17

Re: Bug found in streamts

Beitrag von new.life »

Snakehand hat geschrieben:Can someone with CVS write access please try to fix :-)
...überhaupt eine Reaktion auf Dein post wäre schon ein Erfolg..
Houdini
Developer
Beiträge: 2183
Registriert: Mittwoch 10. Dezember 2003, 07:59

Beitrag von Houdini »

can you try to post a diff or the complete part of the fixed file?
Snakehand
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Mittwoch 7. März 2007, 17:52

Beitrag von Snakehand »

Houdini hat geschrieben:can you try to post a diff or the complete part of the fixed file?
I haven't yet been able to build a new version under Ubuntu, but I have a computer that needs a Linux installation, so I will try to build under openSuse 10.2, to see if that works better.

The change I suggest is to replace theses lines in main()

>>>>>
/* make sure to start with a ts header */
offset = sync_byte_offset(buf, TS_SIZE);

if (offset == -1)
continue;

packet_stdout(buf + offset, r - offset, NULL);
========
packet_stdout(buf, IN_SIZE, NULL);
<<<<<<<<

I have a DM7000 that I can test this on as well, to make sure that this change doesn't break to anything. I think it could be a change in the dmux that makes this a problem on the DM 500 only.

I will submit a patch once I have verified and tested this fix. But I would appreciate if someone could build me a new streamts with this fix, so it can be tested from /var/bin ( editing /var/etc/inetd.conf and restaring inetd with the modified config file makes this easy to test )

Binary can be mailed to fstevenson

I hate spammers

at gmail dot com
Snakehand
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Mittwoch 7. März 2007, 17:52

Beitrag von Snakehand »

Snakehand hat geschrieben: The change I suggest is to replace theses lines in main()

>>>>>
/* make sure to start with a ts header */
offset = sync_byte_offset(buf, TS_SIZE);

if (offset == -1)
continue;

packet_stdout(buf + offset, r - offset, NULL);
========
packet_stdout(buf, IN_SIZE, NULL);
<<<<<<<<
I was provided with a binary that has this modification, and it does improve the stream quality markedly. The stream now will become clean again after a glitch, and not like before continue glitching forever.
Snakehand
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Mittwoch 7. März 2007, 17:52

Beitrag von Snakehand »

Code: Alles auswählen

Index: streamts.c
===================================================================
RCS file: /cvs/tuxbox/apps/dvb/tools/stream/streamts.c,v
retrieving revision 1.19
diff -U3 -r1.19 streamts.c
--- a/streamts.c        21 Jan 2006 22:23:51 -0000      1.19
+++ b/streamts.c        7 Apr 2007 10:35:08 -0000
@@ -49,6 +49,12 @@
 #include <unistd.h>
 #include <config.h>
 #include <ctype.h>
+#include <pthread.h>
+#include <sys/select.h>
+#include <sys/time.h>
+#include <sys/types.h>
+#include <unistd.h>
+
 
 #if HAVE_DVB_API_VERSION < 3
     #include <ost/dmx.h>
@@ -77,7 +83,8 @@
 #endif
 
 /* conversion buffer sizes */
-#define IN_SIZE                (TS_SIZE * 362)
+#define TS_COUNT        (362 * 16)
+#define IN_SIZE                (TS_SIZE * TS_COUNT)
 #define IPACKS         2048
 
 /* demux buffer size */
@@ -89,15 +96,19 @@
 /* tcp packet data size */
 #define PACKET_SIZE    1448
 
+
 unsigned char * buf;
 
 int dvrfd;
 int demuxfd[MAXPIDS];
 
 unsigned char demuxfd_count = 0;
-unsigned char exit_flag = 0;
+volatile unsigned char exit_flag = 0;
 unsigned int writebuf_size = 0;
 unsigned char writebuf[PACKET_SIZE];
+volatile unsigned int read_position = 0;
+volatile unsigned int write_position = 0;
+
 
 static int
 sync_byte_offset (const unsigned char * buf, const unsigned int len) {
@@ -214,6 +225,60 @@
 }
 
 
+void* thread_stream_out( void* vbuffer )
+{
+    // char* buf = (char*)vbuffer;
+    unsigned int start;
+    unsigned int stop;
+    unsigned int todo;
+    unsigned int tmp;
+    ssize_t written;
+    fd_set fs;
+    struct timeval tv;
+    
+    while( !exit_flag )
+    {
+       todo = read_position - write_position;
+       if( todo > 0 ) {
+           start = write_position % TS_COUNT;
+           stop = ( start + todo ) % TS_COUNT;
+           if( start > stop )
+           {
+               tmp = TS_COUNT - start;
+               if( tmp > 0 )
+               {
+                   written = write(STDOUT_FILENO, &buf[ start * TS_SIZE ], tmp*TS_SIZE );
+                   if( written == -1 )
+                   {
+                       exit_flag = 1;
+                   }
+               }
+               start = 0;
+           }
+           tmp = stop - start;
+           if( tmp > 0 )
+           {
+               written = write(STDOUT_FILENO, &buf[ start * TS_SIZE ], tmp*TS_SIZE );
+               if( written == -1 )
+               {
+                   exit_flag = 1;
+               }
+           }
+           write_position += todo;
+       }
+       else {
+           /* wait til data is available for reading */
+           FD_ZERO(&fs);
+           FD_SET( dvrfd, &fs );
+           tv.tv_sec = 0;
+           tv.tv_usec = 10;
+           select( 2 , &fs , NULL, NULL, &tv );
+       }
+    }
+    return NULL;
+}
+
+
 #ifdef TRANSFORM
 static int
 dvr_to_ps (const int dvr_fd, const unsigned short audio_pid, const unsigned short video_pid, const unsigned char ps) {
@@ -349,6 +414,14 @@
 }
 
 
+/*
+ * Thread to read dmux
+ */
+
+
+
+
+
 int
 main (int argc, char ** argv) {
 
@@ -356,10 +429,11 @@
        int pids[MAXPIDS];
        unsigned char *bp;
        unsigned char mode;
-       unsigned char tsfile[IN_SIZE];
+       unsigned char tsfile[1024]; /* Should be MAX_PATH */
        int tsfilelen = 0;
        int fileslice = 0;
        int i = 0;
+       pthread_t write_thread;
 
        if (argc != 2)
                return EXIT_FAILURE;
@@ -467,8 +541,15 @@
        /* write raw transport stream to stdout */
        else 
 #endif
+       if ( mode == 2 )
+       {
+           pthread_create( &write_thread, NULL, thread_stream_out, (void*)buf );
+        }
+
+
        if (mode == 2 || mode == 3) {
                int offset;
+               int i;
 
                ssize_t pos;
                ssize_t r = 0;
@@ -478,8 +559,19 @@
                        pos = 0;
                        todo = IN_SIZE;
 
-                       while (todo) 
+                       if (mode == 2)
+                       {
+                           /* read from demux */
+                           /* read 7 TS blocks at a time, and let the output writing thread catch up */
+                           for( i = 0 ; i < TS_COUNT ; i+= 16 )
+                           {
+                               read(dvrfd, &buf[i*TS_SIZE], TS_SIZE*16);
+                               read_position += 16;
+                           }  
+                       }
+                       else while (todo) 
                        {
+
                                r = read(dvrfd, buf + pos, todo);
                                if (r > 0) {
                                        pos += r;
@@ -487,33 +579,31 @@
                                }
                                else
                                {
-                                       if (mode == 3)
+                                       if (r == -1)
+                                       {
+                                               free(buf);
+                                               return EXIT_FAILURE;
+                                       }
+                                       else
                                        {
-                                               if (r == -1)
-                                               {
+                                               close(dvrfd);
+                                               sprintf(&tsfile[tsfilelen], ".%03d", ++fileslice);
+                                               if ((dvrfd = open(tsfile, O_RDONLY)) < 0) {
                                                        free(buf);
-                                                       return EXIT_FAILURE;
-                                               }
-                                               else
-                                               {
-                                                       close(dvrfd);
-                                                       sprintf(&tsfile[tsfilelen], ".%03d", ++fileslice);
-                                                       if ((dvrfd = open(tsfile, O_RDONLY)) < 0) {
-                                                               free(buf);
-                                                               return EXIT_SUCCESS;
-                                                       }
+                                                       return EXIT_SUCCESS;
                                                }
                                        }
                                }
+                               packet_stdout(buf, IN_SIZE , NULL);
                        }
 
                        /* make sure to start with a ts header */
-                       offset = sync_byte_offset(buf, TS_SIZE);
+                       // offset = sync_byte_offset(buf, TS_SIZE);
+
+                       //if (offset == -1)
+                       //      continue;
+                       // packet_stdout(buf + offset, r - offset, NULL);
 
-                       if (offset == -1)
-                               continue;
-
-                       packet_stdout(buf + offset, r - offset, NULL);
                }
        }
This makes streamts rock solid on DM500

BUT:

This version probably uses a bit to much memory for the circular buffer ( 5% of total DM500 memory ) so it should be tweaked lower,

When I started making this fix, it was on the assumtion that the DM500 network was slow, and the writing to network stalled the dmux reading. Thats wy I now write to the network in a seperate thread. But that assumption about the network beeing slow, might not be the case after all.

After playing with this I think that I can see an underlying problem with the DM500 demux, which I will nned to look into further. Here are som symptoms:

1) ngrab fails to records when bullz text s running
2) Streamts had problems subscribing to streams after channel change
3) Enigma recording over nfs is choppy
4) The fixed version of streamts is overwhelmed by HD streams. The old version would max out CPU load to 100%, the new version uses much less CPU, but still can't keep up with a full HD stream

Symptom 3,4 seems to indicate that there is a serious problem with write buffering from the dmux, besides the stream selection problem that still is a problem in stremts (1,2?). I will have to get into the kernel driver sources in order to learn more
:gruebel:

Some more fiddling seems to indicate that setting the DMX_IMMEDIATE_START flag in setPesFilter() almost eliminates problem (2)
Zuletzt geändert von Snakehand am Montag 9. April 2007, 17:50, insgesamt 3-mal geändert.
new.life
Erleuchteter
Erleuchteter
Beiträge: 797
Registriert: Sonntag 19. Februar 2006, 01:17

Beitrag von new.life »

Snakehand hat geschrieben:This makes streamts rock solid on DM500
Bild
ridax
Beiträge: 1
Registriert: Donnerstag 12. April 2007, 09:06

Re: Bug found in streamts

Beitrag von ridax »

Snakehand hat geschrieben: http://www.ridax.se/dreambox/ describes the problems that this causes on my DM 500C, but does not seem to have understood the GPL when he charges 10 euros for the fix.
Before you claim someone doesn't understand something, maybe YOU should make sure that you understand?

My code is NOT based on the CVS/Tuxbox streamts but a complete write from scratch. If anyone don't believe it, just pay for an independent software developer to compare the sources under NDA.

Until then, you should not use this kind of language...
Snakehand
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Mittwoch 7. März 2007, 17:52

Re: Bug found in streamts

Beitrag von Snakehand »

ridax hat geschrieben:
Snakehand hat geschrieben: http://www.ridax.se/dreambox/ describes the problems that this causes on my DM 500C, but does not seem to have understood the GPL when he charges 10 euros for the fix.
Before you claim someone doesn't understand something, maybe YOU should make sure that you understand?

My code is NOT based on the CVS/Tuxbox streamts but a complete write from scratch. If anyone don't believe it, just pay for an independent software developer to compare the sources under NDA.

Until then, you should not use this kind of language...
As I haven't seen your streamts implementation I could only deduce from your language that you had done the two fixes to the original that you mention. I apologise if my deductions have wronged you in any way, but would like to remind you that this is an open source project, and that I intend to continue developing the linux way. You are of course entiteled to develope closed source style, but then it will be harder for you to implement the same fixes to NFS / CIFS recording as I intend to do.

Cheers.

( BTW I think my language was rather mild )
Snakehand
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Mittwoch 7. März 2007, 17:52

streamts.c status report

Beitrag von Snakehand »

Rather than post an unwieldy diff , I have placed the current state of my source here:
http://traxme.net/streamts.c

Changes are:
  • Multithreaded read/write for aggressive reading of dvr
    ringbuffering
    cleanup in setPesFilter
    Disable Nagling on stdout socket - boosts network performance
    removed broken resync
Some of these changes can also be applied to tuxbox/enigma/lib/dvb/record.cpp And I see no reason why CIFS & NFS recording shouldn't be possible. record.cpp seems to use a 512K buffer, while keeping the default 256k dmx buffer. That can mean that the recording has to a) read 512K from dvr b) write 512K to network - in less time than it takes to capture just 256K of dvr data. I can imagine this will be a problem with a slow NIS or on a HD stream.
:gruebel:
nikipol
Beiträge: 1
Registriert: Donnerstag 12. April 2007, 16:08

Beitrag von nikipol »

Great work Snakehand :)

I tried it out (on a dm500t) and it works very well with only small glitches every now and then.

I noticed that sometimes your streamts doesn't exit, leaving multiple
processes running. This eventually causes the dreambox to freeze.

I don't have much programming experience with the dreambox
but it seems that a PMT filter set on the demux device doesn't return
any data unless the channel is changed a couple of times.

Hope this helps you

nikipol
Snakehand
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Mittwoch 7. März 2007, 17:52

Beitrag von Snakehand »

nikipol hat geschrieben:Great work Snakehand :)

I noticed that sometimes your streamts doesn't exit, leaving multiple
processes running. This eventually causes the dreambox to freeze.

Hope this helps you

nikipol
Thanks, I am still trying to track down the root cause of these problems. The problem with streamts not exiting, is connected to the problem with locking on to the stream, If no data is read from dvr, no data is sendt to the PC and so streamts never detects that the socket has been closed.

I have made some new observations today:
If the DM500 writes as fast as as it can to network ( the same 1MB memory chunk in a loop ) - it will max out at a stable 4.0 megabyte per seconds - with 100% CPU load. So it appears that CPU is the limiting factor for network traffic, and all this talk about changing condensors to the network interface is nonsense.

The other observation that I have made is that high CPU load will cause the demuxing to glitch, and lowering the priority of the writing thread seems to reduce the small glitches from the demuxer. So the original streaming problem looks like they have been caused by the demuxer beeing starved of CPU cycles, triggering the bug that I originally found in streamts.

I am still tweaking buffer sizes and such in streamts for optimal performance, but it also looks like the proper way to fix these issues is to look closer at the demuxing code instead, and if possible optimize the network drivers.

f
Coronas
Developer
Beiträge: 196
Registriert: Dienstag 16. Oktober 2001, 00:00

Re: streamts.c status report

Beitrag von Coronas »

Snakehand hat geschrieben:...Some of these changes can also be applied to tuxbox/enigma/lib/dvb/record.cpp...
Just out of curiosity - what changes might that be? I noticed quite a few problems when using the harddisk on the dbox2 under enigma, and maybe the changes you were thinking about could reduce them as well?
obi
Senior Member
Beiträge: 1282
Registriert: Montag 12. November 2001, 00:00

Beitrag von obi »

Hello Snakehand,

first, I am sorry for the late reply, but technical problems didn't allow me to respond at the time I intended to.

Can you please try this patch with the CVS-Version, just to see whether this already fixes least the sync-issue?

http://www.saftware.de/patches/streamts.diff

It changes two things:

a) Previous versions would call sync_byte_offset() to find the next 0x47 inside the stream. I removed that call as proposed by you.

b) Previous versions would use 'r' as the write length. However, 'r' is reset after every call to read() and multiple read()s can occur until a write() is done. 'pos' should be used instead, which is incremented after every read().

If so, then I'd like to commit this to CVS and kindly ask you to use this version as a base for your work.

The reason is that those two points can easily be adressed by the small patch. Further improvments can still be done later.

Regards,
Obi
dweeb4
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Freitag 28. September 2007, 00:06

Beitrag von dweeb4 »

Hi All,
Just joined this forum. I wondered if this line of enquiry had been resolved - has the streamts been finalised? What's the latest situation?
dweeb4
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Freitag 28. September 2007, 00:06

Beitrag von dweeb4 »

I tried Obi's streamts mods and streaming is almost perfect - when it picks up a channel there are no pixellations or glitches. Only problem seems to be that it doesn't latch onto all channels i.e. those with PCRPid different to VPid

Here's an example:
2 channel streams info - the first one failsto stream (PCRPid not = VPid) & the second works (PCRPid = VPid):

Service Name: Channel 4
Service Provider: Channel 4
Service Reference: 1:0:1:4cc:c:a701:0:0:0:0:
VPID: 121ch (4636d)
APID: 12bch (4796d)
PCRPID: 1218h (4632d)
TPID: 12d8h (4824d)
TSID: 000ch
ONID: a701h
SID: 04cch
PMT: 003bh
Video Format: 704x576 16:9 25 fps
Namespace: 0000h
Supported Crypto Systems: 4a70h Dream Multimedia TV (DreamCrypt)
Used Crypto Systems: 1800h: Kudelski SA, 1801h: Kudelski SA
Frequency: 347 MHz
Symbol Rate: 6887 KSymbols/s
Inversion: Auto
Modulation: 64-QAM
FEC: 3/4

Here's the stream info for a working channel:
Service Name: BBC 1
Service Provider: BBC 1
Service Reference: 1:0:1:4b1:c:a701:0:0:0:0:
VPID: 1201h (4609d)
APID: 12a1h (4769d)
PCRPID: 1201h (4609d)
TPID: 12d1h (4817d)
TSID: 000ch
ONID: a701h
SID: 04b1h
PMT: 0020h
Video Format: 720x576 16:9 25 fps
Namespace: 0000h
Supported Crypto Systems: 4a70h Dream Multimedia TV (DreamCrypt)
Used Crypto Systems: 1800h: Kudelski SA, 1801h: Kudelski SA
Frequency: 347 MHz
Symbol Rate: 6887 KSymbols/s
Inversion: Auto
Modulation: 64-QAM
FEC: 3/4

Just my feedback!
dweeb4
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Freitag 28. September 2007, 00:06

Beitrag von dweeb4 »

I've looked at the streamts source & can't figure where the latching to the channel would happen - I'm no C++ programmer but have programmed in other languages before so can make out the major functionality of the source.

Is there a module which calls streamts with the pids needed for latching onto the channel? Can anybody point me in the right direction?
seife
Developer
Beiträge: 4189
Registriert: Sonntag 2. November 2003, 12:36

Beitrag von seife »

It get's the pids http-style from the client, see the comment at the beginning.

So you probably just need to add the pcrpid to the http-request?
dweeb4
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Freitag 28. September 2007, 00:06

Beitrag von dweeb4 »

Thanks Seife,

I looked at the code before but couldn't see how it read the pids - I will study some more!!
seife
Developer
Beiträge: 4189
Registriert: Sonntag 2. November 2003, 12:36

Beitrag von seife »

It reads them from stdin - since it is started from inetd, it gets the network traffic from stdin. The pids are read between line 418 and 428 - at least if we are both talking about apps/dvb/tools/stream/streamts.c :-)
dweeb4
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Freitag 28. September 2007, 00:06

Beitrag von dweeb4 »

Yes thanks Seife, we are working off the same source.

I can see bp scanned for pids
& PIDS array filled with found pids until maxpids reached or other conditions?

But I don't see anywhere else in source that PIDS array is used other than in line 465 where pes file is written with pids[0] & pids[1]

What & where is the pids array used? I imagined the relevant pids would be needed by VLC to synch audio & video in the stream?
seife
Developer
Beiträge: 4189
Registriert: Sonntag 2. November 2003, 12:36

Beitrag von seife »

Code: Alles auswählen

417                 /* parse stdin / url path, start dmx filters */
418                 do {
419                         sscanf(bp, "%x", &pid);
420
421                         pids[demuxfd_count] = pid;
422
423                         if ((demuxfd[demuxfd_count] = setPesFilter(pid)) < 0)
424                                 break;
425
426                         demuxfd_count++;
427                 }
428                 while ((bp = strchr(bp, ',')) && (bp++) && (demuxfd_count < MAXPIDS));
Line 423 sets the PID filter on the device, no need for the PIDs in another place (apart fromthe ts2ps conversion in line 465).
So the array is not needed at all. Probably it is a leftover from a previous version where someting else was done with the PIDs, i have no idea.
Once those PID filters are set on the device, the stream read from the DVR device contains all those PIDs, so if you need the PCRPID for VDR, you better make sure that you set this filter :-)
dweeb4
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Freitag 28. September 2007, 00:06

Beitrag von dweeb4 »

Thanks again seife,
I PM'ed you as I can't post code here
dweeb4
Neugieriger
Neugieriger
Beiträge: 9
Registriert: Freitag 28. September 2007, 00:06

Beitrag von dweeb4 »

Sorry seife,
It's already in Snakehands source:

Code: Alles auswählen

flt.flags = DMX_IMMEDIATE_START;
Kobazz
Beiträge: 1
Registriert: Donnerstag 28. Februar 2008, 14:16

Re: Bug found in streamts

Beitrag von Kobazz »

I will submit a patch once I have verified and tested this fix. But I would appreciate if someone could build me a new streamts with this fix, so it can be tested from /var/bin ( editing /var/etc/inetd.conf and restaring inetd with the modified config file makes this easy to test )
Well I can't seem to test a modified streamts file this way on Gemini 4.3.1
This is what I did:

Made an init file in /var/etc with this content:

killall -q inetd
killall -q streamts
inetd /var/etc/inetd.conf

modified inetd.conf from /var/etc with this:

streamts stream tcp nowait root /var/bin/streamts streamts -ts
streamtsfile stream tcp nowait root /var/bin/streamts streamts -tsfile

and I've put the modified streamts for testing in /var/bin.

After restarting the box it will load the snakehand version, but not the new file for testing.

Can anyone point me in the right direction. I'm a noob here :gruebel: