From a3caa5974f860b9702089c4ffea54e22f658fb49 Mon Sep 17 00:00:00 2001 From: Andrew de Quincey Date: Sun, 3 Jun 2007 02:43:07 +0100 Subject: [patch] Nasty mmap problem with huge files Hi, I've been tracking down a very odd bug this afternoon. As it turns out it is caused by enabling xine's mmap() support for the input_file.c. I'm running 32 bit linux 2.6.21. The file in question is 0x10e4da000 bytes long (you can probably guess what kind of bug this is by now :) Anyway, the issue stems from the definition of mmap(): void *mmap(void *start, size_t length, int prot, int flags, int fd, off_t offset); compare this to the definition of st_size in struct stat: off_t st_size; /* total size, in bytes */ On my machine (in input_file.c) sizeof(size_t) ==4, whilst sizeof(off_t) == 8. However the compiler doesn't generate a warning when the following is done in xine's code: if ( (this->mmap_base = mmap(NULL, sbuf.st_size, PROT_READ, MAP_SHARED, this->fh, 0)) != (void*)-1 So it silently truncates the upper part of the length. Obviously you cannot mmap() a file that large into (32 bit) memory anyway, but as it turns out, mmapping() 0xe4da000 succeeds, which causes... problems. The patch (against xine-lib 1.1.6) does two things: * Check that the length will not be truncated, while still allowing for mmap()s of large files under 64 bit OSes. * A correctness fix: if mmap() fails, this->mmap_base will be set to 0xffffffff. Later on when the file is closed, this means it was attempting to do munmap(0xffffffff). --- src/input/input_file.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/input/input_file.c b/src/input/input_file.c index fd2b0e733..d57eaacb9 100644 --- a/src/input/input_file.c +++ b/src/input/input_file.c @@ -359,6 +359,9 @@ static int file_plugin_open (input_plugin_t *this_gen ) { file_input_plugin_t *this = (file_input_plugin_t *) this_gen; char *filename; struct stat sbuf; +#ifdef HAVE_MMAP + size_t tmp_size; +#endif lprintf("file_plugin_open\n"); @@ -423,10 +426,14 @@ static int file_plugin_open (input_plugin_t *this_gen ) { } #ifdef HAVE_MMAP - if ( (this->mmap_base = mmap(NULL, sbuf.st_size, PROT_READ, MAP_SHARED, this->fh, 0)) != (void*)-1 ) { + tmp_size = sbuf.st_size; + if ((tmp_size == sbuf.st_size) && + ( (this->mmap_base = mmap(NULL, tmp_size, PROT_READ, MAP_SHARED, this->fh, 0)) != (void*)-1 )) { this->mmap_on = 1; this->mmap_curr = this->mmap_base; this->mmap_len = sbuf.st_size; + } else { + this->mmap_base = NULL; } #endif -- cgit v1.2.3 From f398c68b695ee3dad829d95a724573ca42926205 Mon Sep 17 00:00:00 2001 From: Darren Salt Date: Sun, 3 Jun 2007 03:01:31 +0100 Subject: Add a comment & changelog entry for the mmap bug fix. --- src/input/input_file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/input/input_file.c b/src/input/input_file.c index d57eaacb9..c755a84a8 100644 --- a/src/input/input_file.c +++ b/src/input/input_file.c @@ -426,7 +426,7 @@ static int file_plugin_open (input_plugin_t *this_gen ) { } #ifdef HAVE_MMAP - tmp_size = sbuf.st_size; + tmp_size = sbuf.st_size; /* may cause truncation - if it does, DON'T mmap! */ if ((tmp_size == sbuf.st_size) && ( (this->mmap_base = mmap(NULL, tmp_size, PROT_READ, MAP_SHARED, this->fh, 0)) != (void*)-1 )) { this->mmap_on = 1; -- cgit v1.2.3 From 3e2ed6d0263c021644041ad3928ba4e7074360d7 Mon Sep 17 00:00:00 2001 From: Andrew de Quincey Date: Sun, 3 Jun 2007 17:47:19 +0100 Subject: [patch] Fix video pid misdetection Hi, the next bug that has been annoying me is as follows. I have some streams recorded from BBC4 on UK DVB-T. BBC4 only actually starts transmitting at about 7pm; prior to that there is a static picture saying it is not playing just now. With these streams and xine, I would get audio, but no picture. Looking at the PMT table during the static picture, all the streams have type 0x0b. However there IS a video stream in there, but there are also several streams of binary data. Xine's current video PID auto-detection code was locking on to one of these streams of binary data because it contained the magic sequence 00 00 01 e0 at one of the PUS. *HOWEVER* it is NOT a PES stream; this sequence is just an accident. The other problem is that xine can only handle one video stream; so once it was misdetected once, it was stuck at that PID. The attached patch changes the corrupted_pes flag into a counter. If a video stream has more than CORRUPT_PES_THRESHOLD corrupt PES packets in a row, then it is deselected as the video stream, and auto-detection is kicked off again. Auto-detection will now also ignore any streams seen previously which have a nonzero corrupted_pes count. This works very well; I can now see the video fine. One possible issue might be that if you get a lot of corrupt PES in a stream which really IS the video stream, it will be deselected. However, this is not actually a problem: once the corruption goes away, the corrupted_pes counter will be reset to 0, and the stream will once again be autodetected as the video stream (and playback will continue from there). --- src/demuxers/demux_ts.c | 41 +++++++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/demuxers/demux_ts.c b/src/demuxers/demux_ts.c index f2da5f268..57925deff 100644 --- a/src/demuxers/demux_ts.c +++ b/src/demuxers/demux_ts.c @@ -180,6 +180,8 @@ #define MAX_PES_BUF_SIZE 2048 +#define CORRUPT_PES_THRESHOLD 10 + #define NULL_PID 0x1fff #define INVALID_PID ((unsigned int)(-1)) #define INVALID_PROGRAM ((unsigned int)(-1)) @@ -913,12 +915,17 @@ static void demux_ts_buffer_pes(demux_ts_t*this, unsigned char *ts, m->buf = m->fifo->buffer_pool_alloc(m->fifo); if (!demux_ts_parse_pes_header(this->stream->xine, m, ts, len, this->stream)) { - m->corrupted_pes = 1; m->buf->free_buffer(m->buf); m->buf = NULL; - xprintf(this->stream->xine, XINE_VERBOSITY_DEBUG, + + if (m->corrupted_pes > CORRUPT_PES_THRESHOLD) { + if (this->videoPid == m->pid) + this->videoPid = INVALID_PID; + } else { + m->corrupted_pes++; + xprintf(this->stream->xine, XINE_VERBOSITY_DEBUG, "demux_ts: PID 0x%.4x: corrupted pes encountered\n", m->pid); - + } } else { m->corrupted_pes = 0; @@ -1784,13 +1791,27 @@ static void demux_ts_parse_packet (demux_ts_t*this) { if ( (pes_stream_id >= VIDEO_STREAM_S) && (pes_stream_id <= VIDEO_STREAM_E) ) { if ( this->videoPid == INVALID_PID) { - - xprintf (this->stream->xine, XINE_VERBOSITY_DEBUG, - "demux_ts: auto-detected video pid 0x%.4x\n", pid); - - this->videoPid = pid; - this->videoMedia = this->media_num; - demux_ts_pes_new(this, this->media_num++, pid, this->video_fifo, pes_stream_id); + int i, found = 0; + for(i = 0; i < this->media_num; i++) { + if (this->media[i].pid == pid) { + found = 1; + break; + } + } + + if (found && (this->media[i].corrupted_pes == 0)) { + this->videoPid = pid; + this->videoMedia = i; + } else if (!found) { + this->videoPid = pid; + this->videoMedia = this->media_num; + demux_ts_pes_new(this, this->media_num++, pid, this->video_fifo, pes_stream_id); + } + + if (this->videoPid != INVALID_PID) { + xprintf (this->stream->xine, XINE_VERBOSITY_DEBUG, + "demux_ts: auto-detected video pid 0x%.4x\n", pid); + } } } else if ( (pes_stream_id >= AUDIO_STREAM_S) && (pes_stream_id <= AUDIO_STREAM_E) ) { if (this->audio_tracks_count < MAX_AUDIO_TRACKS) { -- cgit v1.2.3