From c29f5163db85b1b4097a791ca1ba96f2b52f1f04 Mon Sep 17 00:00:00 2001 From: Simon Farnsworth Date: Fri, 13 Jul 2007 15:41:12 +0100 Subject: Prevent ticket system deadlock when using DVB subtitles When using DVB subtitles on an SMP machine, we see occasional lockups, which appear to be caused by one thread acquiring the same ticket twice. Fix this, by preventing acquire() and release() from blocking if the current thread has already acquired the ticket. Code sequences like the following can still block in all acquires and releases: ticket->acquire(...) /* Do something */ ticket->release(...) However, code sequences like the following, which used to deadlock if ticket was revoked at just the wrong moment, now succeed: ticket->acquire(...) /* Do something */ ticket->acquire(...) /* This acquire cannot block */ /* Do something */ ticket->release(...) /* This release cannot block */ /* Do something */ ticket->release(...) Without this patch, the inner acquire() and release() calls could block if ticket was revoked at the wrong time. revoke() would not unblock the blocking acquire until there have been as many release()s as acquire()s, which cannot happen. --- src/xine-engine/xine.c | 63 +++++++++++++++++++++++++++++++++++++++-- src/xine-engine/xine_internal.h | 6 ++++ 2 files changed, 66 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/xine-engine/xine.c b/src/xine-engine/xine.c index 254d596b1..00f1dc41c 100644 --- a/src/xine-engine/xine.c +++ b/src/xine-engine/xine.c @@ -127,17 +127,54 @@ void _x_extra_info_merge( extra_info_t *dst, extra_info_t *src ) { } } +static int acquire_allowed_to_block(xine_ticket_t *this) { + pthread_t own_id = pthread_self(); + unsigned entry; + unsigned new_size; + + for(entry = 0; entry < this->holder_thread_count; ++entry) { + if(this->holder_threads[entry].holder == own_id) { + /* This thread may already hold this ticket */ + this->holder_threads[entry].count++; + return (this->holder_threads[entry].count == 1); + } + } + /* If we get this far, this thread hasn't claimed this ticket before. + We need to give it a new entry in the list, then return true */ + for(entry = 0; entry < this->holder_thread_count; ++entry) { + if(this->holder_threads[entry].count == 0) { + this->holder_threads[entry].holder = own_id; + this->holder_threads[entry].count = 1; + return 1; + } + } + /* List too small. Realloc to larger size */ + new_size = this->holder_thread_count * 2; + lprintf("Reallocing from %d to %d entries\n", this->holder_thread_count, new_size); + + this->holder_threads = realloc(this->holder_threads, sizeof(*this->holder_threads) * new_size); + memset(this->holder_threads + this->holder_thread_count, 0, this->holder_thread_count); + + /* Old size is equivalent to index of first newly allocated entry*/ + this->holder_threads[this->holder_thread_count].count = 1; + this->holder_threads[this->holder_thread_count].holder = own_id; + this->holder_thread_count = new_size; + + return 1; +} + static int ticket_acquire_internal(xine_ticket_t *this, int irrevocable, int nonblocking) { int must_wait = 0; pthread_mutex_lock(&this->lock); + int allowed_to_block = acquire_allowed_to_block(this); if (this->ticket_revoked && !this->irrevocable_tickets) must_wait = !nonblocking; else if (this->atomic_revoke && !pthread_equal(this->atomic_revoker_thread, pthread_self())) must_wait = 1; - if (must_wait) { + if (must_wait && allowed_to_block) { if (nonblocking) { pthread_mutex_unlock(&this->lock); return 0; @@ -162,9 +199,25 @@ static void ticket_acquire(xine_ticket_t *this, int irrevocable) { ticket_acquire_internal(this, irrevocable, 0); } +static int release_allowed_to_block(xine_ticket_t *this) { + pthread_t own_id = pthread_self(); + unsigned entry; + + for(entry = 0; entry < this->holder_thread_count; ++entry) { + if(this->holder_threads[entry].holder == own_id) { + this->holder_threads[entry].count--; + return this->holder_threads[entry].count == 0; + } + } + lprintf("BUG! Ticket 0x%p released by a thread that never took it! Allowing code to continue\n", this); + _x_assert(0); + return 1; +} + static void ticket_release_internal(xine_ticket_t *this, int irrevocable, int nonblocking) { pthread_mutex_lock(&this->lock); + int allowed_to_block = release_allowed_to_block(this); this->tickets_granted--; if (irrevocable) @@ -172,8 +225,10 @@ static void ticket_release_internal(xine_ticket_t *this, int irrevocable, int no if (this->ticket_revoked && !this->tickets_granted) pthread_cond_broadcast(&this->revoked); - if (this->ticket_revoked && !this->irrevocable_tickets && !nonblocking) - pthread_cond_wait(&this->issued, &this->lock); + if (allowed_to_block) { + if (this->ticket_revoked && !this->irrevocable_tickets && !nonblocking) + pthread_cond_wait(&this->issued, &this->lock); + } pthread_mutex_unlock(&this->lock); } @@ -262,6 +317,8 @@ static xine_ticket_t *ticket_init(void) { port_ticket->issue = ticket_issue; port_ticket->revoke = ticket_revoke; port_ticket->dispose = ticket_dispose; + port_ticket->holder_thread_count = XINE_MAX_TICKET_HOLDER_THREADS; + port_ticket->holder_threads = calloc(XINE_MAX_TICKET_HOLDER_THREADS,sizeof(*port_ticket->holder_threads)); pthread_mutex_init(&port_ticket->lock, NULL); pthread_mutex_init(&port_ticket->revoke_lock, NULL); diff --git a/src/xine-engine/xine_internal.h b/src/xine-engine/xine_internal.h index 5f9a82f97..5523001ca 100644 --- a/src/xine-engine/xine_internal.h +++ b/src/xine-engine/xine_internal.h @@ -75,6 +75,7 @@ extern "C" { #define XINE_MAX_EVENT_LISTENERS 50 #define XINE_MAX_EVENT_TYPES 100 +#define XINE_MAX_TICKET_HOLDER_THREADS 64 /* used by plugin loader */ #define XINE_VERSION_CODE XINE_MAJOR_VERSION*10000+XINE_MINOR_VERSION*100+XINE_SUB_VERSION @@ -179,6 +180,11 @@ struct xine_ticket_s { int pending_revocations; int atomic_revoke; pthread_t atomic_revoker_thread; + struct { + int count; + pthread_t holder; + } *holder_threads; + unsigned holder_thread_count; #endif }; -- cgit v1.2.3 From 7cb04769574d9ef849ca7a7bd4da45671639eb77 Mon Sep 17 00:00:00 2001 From: Simon Farnsworth Date: Thu, 12 Jul 2007 11:30:14 +0100 Subject: Fix memory leak in video_overlay.c When running DVB subtitles for a long period of time (over 24 hours), we noticed a slow leak of memory. This patch removes one cause of leakage for us. --- src/xine-engine/video_overlay.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/xine-engine/video_overlay.c b/src/xine-engine/video_overlay.c index 231aa5a70..574f42ac3 100644 --- a/src/xine-engine/video_overlay.c +++ b/src/xine-engine/video_overlay.c @@ -395,6 +395,8 @@ static int video_overlay_event( video_overlay_t *this, int64_t vpts ) { #endif /* free any overlay associated with this event */ if (this->events[this_event].event->object.overlay != NULL) { + if( this->events[this_event].event->object.overlay->rle != NULL ) + free( this->events[this_event].event->object.overlay->rle ); free(this->events[this_event].event->object.overlay); this->events[this_event].event->object.overlay = NULL; } @@ -406,9 +408,11 @@ static int video_overlay_event( video_overlay_t *this, int64_t vpts ) { printf ("video_overlay: FREE SPU NOW\n"); #endif /* free any overlay associated with this event */ - if (this->events[this_event].event->object.overlay != NULL) { + if( this->events[this_event].event->object.overlay != NULL) { + if( this->events[this_event].event->object.overlay->rle != NULL ) + free( this->events[this_event].event->object.overlay->rle ); free(this->events[this_event].event->object.overlay); - this->events[this_event].event->object.overlay = NULL; + this->events[this_event].event->object.overlay = NULL; } /* this avoid removing this_event from the queue * (it will be removed at the end of this loop) */ -- cgit v1.2.3 From 33d32c6238d0d5d9ae00a2fee6bba2c987ff0f21 Mon Sep 17 00:00:00 2001 From: Simon Farnsworth Date: Thu, 12 Jul 2007 11:29:42 +0100 Subject: Fix thread leak in DVB subtitles, and enhance spec compliance When leaving xine playing DVB with subtitles for a long period of time, I noticed a gradual increase in memory use, caused by it creating more and more timeout threads. In addition, the existing thread was not safe w.r.t destruction of the decoder, and would occasionally segfault xine. Further, EN 300 743 states that the timeout should be sent by the broadcaster; the existing thread had a constant 6 second timeout, whereas (e.g.) BBC NEWS 24 subtitles are broadcast with a 15 second timeout. In theory, this could result in subtitles being hidden in error. This rework changes the thread to pick up a timeout set by draw_subtitles; in addition, it uses pthread condition variables to avoid any need to kill and recreate the thread. --- src/libspudvb/xine_spudvb_decoder.c | 139 ++++++++++++++++++++---------------- 1 file changed, 76 insertions(+), 63 deletions(-) (limited to 'src') diff --git a/src/libspudvb/xine_spudvb_decoder.c b/src/libspudvb/xine_spudvb_decoder.c index f2fcfe182..339d66b2e 100644 --- a/src/libspudvb/xine_spudvb_decoder.c +++ b/src/libspudvb/xine_spudvb_decoder.c @@ -28,15 +28,11 @@ */ #include "pthread.h" +#include #include "xine_internal.h" #include "osd.h" #define MAX_REGIONS 7 -/* check every DVBSUB_TIMER_DELAY seconds */ -#define DVBSUB_TIMER_DELAY 1 -/* hide subs after n counts of the delay */ -#define SUB_TIMEOUT 6 - typedef struct { int x, y; unsigned char is_visible; @@ -90,6 +86,9 @@ typedef struct dvb_spu_decoder_s { spu_dvb_descriptor_t *spu_descriptor; + /* dvbsub_osd_mutex should be locked around all calls to this->osd_renderer->show() + and this->osd_renderer->hide() */ + pthread_mutex_t dvbsub_osd_mutex; osd_object_t *osd; char *bitmap; @@ -101,11 +100,9 @@ typedef struct dvb_spu_decoder_s { uint64_t vpts; uint64_t end_vpts; - pthread_mutex_t dvbsub_timer_mutex; - /* This is set to non-zero if the timer thread is wanted to stop. */ - int dvbsub_timer_stop; pthread_t dvbsub_timer_thread; - unsigned int dvbsub_timer_tcount; + struct timespec dvbsub_hide_timeout; + pthread_cond_t dvbsub_restart_timeout; dvbsub_func_t *dvbsub; int show; } dvb_spu_decoder_t; @@ -544,43 +541,58 @@ void process_object_data_segment (dvb_spu_decoder_t * this) } } - -/* Sleep routine for pthread */ -static void dvbsub_pthread_sleep(int seconds) { - pthread_mutex_t dummy_mutex; - pthread_cond_t dummy_cond; - struct timespec timeout; - - /* Create a dummy mutex which doesn't unlock for sure while waiting. */ - pthread_mutex_init(&dummy_mutex, NULL); - pthread_mutex_lock(&dummy_mutex); - - /* Create a dummy condition variable. */ - pthread_cond_init(&dummy_cond, NULL); - - timeout.tv_sec = time(NULL) + seconds; - timeout.tv_nsec = 0; - - pthread_cond_timedwait(&dummy_cond, &dummy_mutex, &timeout); - - pthread_cond_destroy(&dummy_cond); - pthread_mutex_unlock(&dummy_mutex); - pthread_mutex_destroy(&dummy_mutex); +static void unlock_mutex_cancellation_func(void *mutex_gen) +{ + pthread_mutex_t *mutex = (pthread_mutex_t*) mutex_gen; + pthread_mutex_unlock(mutex); } +/* Thread routine that checks for subtitle timeout periodically. + To avoid unexpected subtitle hiding, calls to this->stream->osd_renderer->show() + should be in blocks like: -/* Thread routine that checks for subtitle timeout periodically. */ -static void* dvbsub_timer_func(void *this_gen) { - dvb_spu_decoder_t *this = (dvb_spu_decoder_t *) this_gen; + pthread_mutex_lock(&this->dvbsub_osd_mutex); + this->stream->osd_renderer->show(...); + this->dvbsub_hide_timeout.tv_sec = time(NULL) + timeout value; + pthread_cond_signal(&this->dvbsub_restart_timeout); + pthread_mutex_unlock(&this->dvbsub_osd_mutex); - while (!this->dvbsub_timer_stop) { - pthread_mutex_lock(&this->dvbsub_timer_mutex); - if(this->dvbsub_timer_tcount++ > SUB_TIMEOUT) - this->stream->osd_renderer->hide (this->osd, 0); - pthread_mutex_unlock(&this->dvbsub_timer_mutex); - dvbsub_pthread_sleep(DVBSUB_TIMER_DELAY); + This ensures that the timeout is changed with the lock held, and + that the thread is signalled to pick up the new timeout. +*/ +static void* dvbsub_timer_func(void *this_gen) +{ + dvb_spu_decoder_t *this = (dvb_spu_decoder_t *) this_gen; + pthread_mutex_lock(&this->dvbsub_osd_mutex); + + /* If we're cancelled via pthread_cancel, unlock the mutex */ + pthread_cleanup_push(unlock_mutex_cancellation_func, &this->dvbsub_osd_mutex); + + while(1) + { + /* Record the current timeout, and wait - note that pthread_cond_timedwait + will unlock the mutex on entry, and lock it on exit */ + struct timespec timeout = this->dvbsub_hide_timeout; + int result = pthread_cond_timedwait(&this->dvbsub_restart_timeout, + &this->dvbsub_osd_mutex, + &this->dvbsub_hide_timeout); + if(result == ETIMEDOUT && + timeout.tv_sec == this->dvbsub_hide_timeout.tv_sec && + timeout.tv_nsec == this->dvbsub_hide_timeout.tv_nsec) + { + /* We timed out, and no-one changed the timeout underneath us. + Hide the OSD, then wait until we're signalled. */ + if(this && this->stream && this->stream->osd_renderer && this->osd) + { + lprintf("Hiding OSD in emergency thread\n"); + this->stream->osd_renderer->hide(this->osd, 0); + } + pthread_cond_wait(&this->dvbsub_restart_timeout, &this->dvbsub_osd_mutex); } - return NULL; + } + + pthread_cleanup_pop(1); + return NULL; } void draw_subtitles (dvb_spu_decoder_t * this) @@ -614,23 +626,17 @@ void draw_subtitles (dvb_spu_decoder_t * this) if(display){ - /* start timer thread if stopped */ - if(this->dvbsub_timer_stop){ - this->dvbsub_timer_stop=0; - if (pthread_create(&this->dvbsub_timer_thread, NULL, dvbsub_timer_func, this) != 0) { - xprintf(this->class->xine, XINE_VERBOSITY_DEBUG, _("dvbsub: cannot create timer thread\n")); - } - } - /* display immediately at requested PTS*/ this->stream->osd_renderer->set_palette(this->osd,(uint32_t *)this->dvbsub->colours,this->dvbsub->trans); this->stream->osd_renderer->draw_bitmap (this->osd,this->bitmap, 1,1,720,576,NULL); - pthread_mutex_lock(&this->dvbsub_timer_mutex); + pthread_mutex_lock(&this->dvbsub_osd_mutex); this->stream->osd_renderer->show (this->osd, this->vpts); - /* reset the timer thread */ - this->dvbsub_timer_tcount=0; - pthread_mutex_unlock(&this->dvbsub_timer_mutex); + this->dvbsub_hide_timeout.tv_nsec = 0; + this->dvbsub_hide_timeout.tv_sec = time(NULL) + this->dvbsub->page.page_time_out; + lprintf("page_time_out %d\n",this->dvbsub->page.page_time_out); + pthread_cond_signal(&this->dvbsub_restart_timeout); + pthread_mutex_unlock(&this->dvbsub_osd_mutex); } } @@ -651,8 +657,10 @@ static void spudec_decode_data (spu_decoder_t * this_gen, buf_element_t * buf) if (buf->decoder_flags & BUF_FLAG_SPECIAL) { if (buf->decoder_info[1] == BUF_SPECIAL_SPU_DVB_DESCRIPTOR) { if (buf->decoder_info[2] == 0) { - this->dvbsub_timer_stop=1; - this->stream->osd_renderer->hide (this->osd, 0); + /* Hide the osd - note that if the timeout thread times out, it'll rehide, which is harmless */ + pthread_mutex_lock(&this->dvbsub_osd_mutex); + this->stream->osd_renderer->hide(this->osd, 0); + pthread_mutex_unlock(&this->dvbsub_osd_mutex); } else { xine_fast_memcpy (this->spu_descriptor, buf->decoder_info_ptr[2], buf->decoder_info[2]); @@ -746,8 +754,10 @@ static void spudec_reset (spu_decoder_t * this_gen) { dvb_spu_decoder_t *this = (dvb_spu_decoder_t *) this_gen; - if (this->osd) - this->stream->osd_renderer->hide (this->osd, 0); + /* Hide the osd - if the timeout thread times out, it'll rehide harmlessly */ + pthread_mutex_lock(&this->dvbsub_osd_mutex); + this->stream->osd_renderer->hide(this->osd, 0); + pthread_mutex_unlock(&this->dvbsub_osd_mutex); } @@ -760,9 +770,10 @@ static void spudec_dispose (spu_decoder_t * this_gen) { dvb_spu_decoder_t *this = (dvb_spu_decoder_t *) this_gen; - if(!this->dvbsub_timer_stop){ - this->dvbsub_timer_stop=1; - } + pthread_cancel(this->dvbsub_timer_thread); + pthread_join(this->dvbsub_timer_thread, NULL); + pthread_mutex_destroy(&this->dvbsub_osd_mutex); + pthread_cond_destroy(&this->dvbsub_restart_timeout); if(this->spu_descriptor){ free(this->spu_descriptor); @@ -822,9 +833,11 @@ static spu_decoder_t *dvb_spu_class_open_plugin (spu_decoder_class_t * class_gen this->stream->osd_renderer->set_encoding (this->osd, NULL); this->stream->osd_renderer->set_text_palette (this->osd, TEXTPALETTE_YELLOW_BLACK_TRANSPARENT, OSD_TEXT1); - - /* subtitle timer thread. */ - this->dvbsub_timer_stop = 1; + pthread_mutex_init(&this->dvbsub_osd_mutex, NULL); + pthread_cond_init(&this->dvbsub_restart_timeout, NULL); + this->dvbsub_hide_timeout.tv_nsec = 0; + this->dvbsub_hide_timeout.tv_sec = time(NULL); + pthread_create(&this->dvbsub_timer_thread, NULL, dvbsub_timer_func, this); return (spu_decoder_t *) this; } -- cgit v1.2.3 From 198fa979f89ba74743a301816d38b46a73770856 Mon Sep 17 00:00:00 2001 From: Simon Farnsworth Date: Thu, 12 Jul 2007 11:28:43 +0100 Subject: Remove realloc from osd.c to prevent memory leak due to fragmentation show() in osd.c uses realloc in an effort to minimise the amount of memory actually used for rle objects. In practice, this caused xine to fragment memory, and gradually use more and more RAM (measured over a period of 24 to 72 hours). Change osd.c to allocate the maximum amount of memory it could need; because it touches this memory in a linear fashion, lazy page allocation will ensure that most of the memory used is needed. Further, because this makes the per-drawing allocations the same size, it avoids virtual address space fragmentation. --- src/xine-engine/osd.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/xine-engine/osd.c b/src/xine-engine/osd.c index c4709c8ab..ebc12300b 100644 --- a/src/xine-engine/osd.c +++ b/src/xine-engine/osd.c @@ -197,7 +197,7 @@ static int _osd_show (osd_object_t *osd, int64_t vpts, int unscaled ) { osd_renderer_t *this = osd->renderer; video_overlay_manager_t *ovl_manager; rle_elem_t rle, *rle_p=0; - int x, y, required; + int x, y; uint8_t *c; lprintf("osd=%p vpts=%"PRId64"\n", osd, vpts); @@ -251,11 +251,11 @@ static int _osd_show (osd_object_t *osd, int64_t vpts, int unscaled ) { this->event.object.overlay->hili_right = this->event.object.overlay->width; /* there will be at least that many rle objects (one for each row) */ - required = osd->y2 - osd->y1; this->event.object.overlay->num_rle = 0; - this->event.object.overlay->data_size = 1024; - while (required > this->event.object.overlay->data_size) - this->event.object.overlay->data_size += 1024; + /* We will never need more rle objects than columns in any row + Rely on lazy page allocation to avoid us actually taking up + this much RAM */ + this->event.object.overlay->data_size = osd->width * osd->height; rle_p = this->event.object.overlay->rle = malloc(this->event.object.overlay->data_size * sizeof(rle_elem_t) ); @@ -272,14 +272,6 @@ static int _osd_show (osd_object_t *osd, int64_t vpts, int unscaled ) { /* loop over the remaining pixels in the row */ for( x = osd->x1 + rle.len; x < osd->x2; x++, c++ ) { if( rle.color != *c ) { - if( (this->event.object.overlay->num_rle + required) > - this->event.object.overlay->data_size ) { - this->event.object.overlay->data_size += 1024; - rle_p = this->event.object.overlay->rle = - realloc( this->event.object.overlay->rle, - this->event.object.overlay->data_size * sizeof(rle_elem_t) ); - rle_p += this->event.object.overlay->num_rle; - } #ifdef DEBUG_RLE lprintf("(%d, %d), ", rle.len, rle.color); #endif @@ -297,8 +289,6 @@ static int _osd_show (osd_object_t *osd, int64_t vpts, int unscaled ) { #endif *rle_p++ = rle; this->event.object.overlay->num_rle++; - /* another row done */ - required--; } #ifdef DEBUG_RLE lprintf("osd_show %p rle ends\n", osd); -- cgit v1.2.3 From 7aa6fcf6f8947faa67e095c8decff779673de17f Mon Sep 17 00:00:00 2001 From: Simon Farnsworth Date: Thu, 12 Jul 2007 11:24:27 +0100 Subject: Simplify input_rtp locking We have seen input_rtp lock up in use, and traced the problem to the separate tail/head locks on the input buffer. Reduce to a single lock, increasing lock contention between the reader and the writer, but removing the previous deadlock risk. Also use select() before recv(), to ensure that we never wait forever for packets (e.g. if we're trying to receive a multicast stream, but an administrator has blocked all multicast packets to the device - iptables -A INPUT --dst 224.0.0.0/4 -j DROP induces this failure for testing). --- src/input/input_rtp.c | 193 ++++++++++++++++++++++++++++---------------------- 1 file changed, 108 insertions(+), 85 deletions(-) (limited to 'src') diff --git a/src/input/input_rtp.c b/src/input/input_rtp.c index d4ba804c6..681bced0f 100644 --- a/src/input/input_rtp.c +++ b/src/input/input_rtp.c @@ -79,6 +79,7 @@ #include #include #include +#include #if defined (__SVR4) && defined (__sun) # include @@ -125,11 +126,9 @@ typedef struct { int fh; unsigned char *buffer; /* circular buffer */ - unsigned char *buffer_tail; /* tail pointer used by reader */ - unsigned char *buffer_head; /* head pointer used by writer */ + unsigned char *buffer_get_ptr; /* get pointer used by reader */ + unsigned char *buffer_put_ptr; /* put pointer used by writer */ long buffer_count; /* number of bytes in the buffer */ - pthread_mutex_t buffer_mutex; /* only used for locking the - * the buffer count variable */ unsigned char packet_buffer[65536]; @@ -143,13 +142,12 @@ typedef struct { char preview[MAX_PREVIEW_SIZE]; int preview_size; + int preview_read_done; /* boolean true after attempt to read input stream for preview */ nbc_t *nbc; - pthread_mutex_t writer_mut; + pthread_mutex_t buffer_ring_mut; pthread_cond_t writer_cond; - - pthread_mutex_t reader_mut; pthread_cond_t reader_cond; } rtp_input_plugin_t; @@ -198,7 +196,7 @@ static int host_connect_attempt(struct in_addr ia, int port, /* Try to increase receive buffer to 1MB to avoid dropping packets */ - optval = 1024 * 1024; + optval = BUFFER_SIZE; if ((setsockopt(s, SOL_SOCKET, SO_RCVBUF, &optval, sizeof(optval))) < 0) { LOG_MSG(xine, _("setsockopt(SO_RCVBUF): %s.\n"), strerror(errno)); @@ -298,6 +296,7 @@ static void * input_plugin_read_loop(void *arg) { rtp_input_plugin_t *this = (rtp_input_plugin_t *) arg; unsigned char *data; long length; + fd_set read_fds; while (1) { @@ -308,8 +307,28 @@ static void * input_plugin_read_loop(void *arg) { */ pthread_testcancel(); - length = recv(this->fh, this->packet_buffer, - sizeof(this->packet_buffer), 0); + { + struct timeval recv_timeout; + int rc; + + recv_timeout.tv_sec = 2; + recv_timeout.tv_usec = 0; + + FD_ZERO( &read_fds ); + FD_SET( this->fh, &read_fds ); + + /* wait for a packet to arrive - but do not hang! */ + rc = select( this->fh+1, &read_fds, NULL, NULL, &recv_timeout ); + if( rc > 0 ) + { + length = recv(this->fh, this->packet_buffer, + sizeof(this->packet_buffer), 0); + } + else if( rc == 0 ) + length = 0; + else + length = -1; + } pthread_testcancel(); if (length < 0) { @@ -362,28 +381,31 @@ static void * input_plugin_read_loop(void *arg) { } /* insert data into cyclic buffer */ - while (length > 0) { - - /* work with a copy of buffer count, while the variable can - * be updated by the reader - */ - - long buffer_count = this->buffer_count; - long n; + if (length > 0) { /* * if the buffer is full, wait for the reader * to signal */ - if(buffer_count >= BUFFER_SIZE) { - pthread_mutex_lock(&this->writer_mut); - pthread_cond_wait(&this->writer_cond, &this->writer_mut); - pthread_mutex_unlock(&this->writer_mut); - /* update the buffer count again */ - buffer_count = this->buffer_count; - } - + pthread_mutex_lock(&this->buffer_ring_mut); + /* wait for enough space to write the whole of the recv'ed data */ + while( (BUFFER_SIZE - this->buffer_count) < length ) + { + struct timeval tv; + struct timespec timeout; + + gettimeofday(&tv, NULL); + + timeout.tv_nsec = tv.tv_usec * 1000; + timeout.tv_sec = tv.tv_sec + 2; + + if( pthread_cond_timedwait(&this->writer_cond, &this->buffer_ring_mut, &timeout) != 0 ) + { + fprintf( stdout, "input_rtp: buffer ring not read within 2 seconds!\n" ); + } + } + /* Now there's enough space to write some bytes into the buffer * determine how many bytes can be written. If the buffer wraps * around, write in two pieces: from the head pointer to the @@ -391,37 +413,29 @@ static void * input_plugin_read_loop(void *arg) { * of bytes. */ - if(length > (BUFFER_SIZE - buffer_count)) { - n = BUFFER_SIZE - buffer_count; - } - else { - n = length; - } - - if(((this->buffer_head - this->buffer) + n) > BUFFER_SIZE) { - n = BUFFER_SIZE - (this->buffer_head - this->buffer); - } - - /* The actual write... */ - memcpy(this->buffer_head, data, n); + { + long buffer_space_remaining = BUFFER_SIZE - (this->buffer_put_ptr - this->buffer); + + if( buffer_space_remaining >= length ) + { + /* data fits inside the buffer */ + memcpy(this->buffer_put_ptr, data, length); + this->buffer_put_ptr += length; + } + else + { + /* data wrapped around the end of the buffer */ + memcpy(this->buffer_put_ptr, data, buffer_space_remaining); + memcpy(this->buffer, &data[buffer_space_remaining], length - buffer_space_remaining); + this->buffer_put_ptr = &this->buffer[ length - buffer_space_remaining ]; + } + } - data += n; - length -= n; - - /* update head pointer; and check for wrap around */ - this->buffer_head += n; - if(this->buffer_head - this->buffer >= BUFFER_SIZE) - this->buffer_head = this->buffer; - - /* lock the mutex; for updating the count */ - pthread_mutex_lock(&this->buffer_mutex); - this->buffer_count += n; - pthread_mutex_unlock(&this->buffer_mutex); + this->buffer_count += length; /* signal the reader that there is new data */ - pthread_mutex_lock(&this->reader_mut); pthread_cond_signal(&this->reader_cond); - pthread_mutex_unlock(&this->reader_mut); + pthread_mutex_unlock(&this->buffer_ring_mut); } } } @@ -443,33 +457,25 @@ static off_t rtp_plugin_read (input_plugin_t *this_gen, off_t n; - /* work with a copy of the buffer count, while the variable can - * be updated by the writer - */ + pthread_mutex_lock(&this->buffer_ring_mut); - long buffer_count = this->buffer_count; - /* * if nothing in the buffer, wait for data for 5 seconds. If * no data is received within this timeout, return the number * of bytes already received (which is likely to be 0) */ - if(buffer_count == 0) { + if(this->buffer_count == 0) { gettimeofday(&tv, NULL); timeout.tv_nsec = tv.tv_usec * 1000; timeout.tv_sec = tv.tv_sec + 5; - pthread_mutex_lock(&this->reader_mut); - if(pthread_cond_timedwait(&this->reader_cond, &this->reader_mut, &timeout) != 0) + if(pthread_cond_timedwait(&this->reader_cond, &this->buffer_ring_mut, &timeout) != 0) { /* we timed out, no data available */ - pthread_mutex_unlock(&this->reader_mut); + pthread_mutex_unlock(&this->buffer_ring_mut); return copied; } - pthread_mutex_unlock(&this->reader_mut); - /* update the local buffer count variable again */ - buffer_count = this->buffer_count; } /* Now determine how many bytes can be read. If the buffer @@ -478,38 +484,34 @@ static off_t rtp_plugin_read (input_plugin_t *this_gen, * update the buffer count. Finally read the second piece * from the base to the remaining count */ - if(length > buffer_count) { - n = buffer_count; + if(length > this->buffer_count) { + n = this->buffer_count; } else { n = length; } - if(((this->buffer_tail - this->buffer) + n) > BUFFER_SIZE) { - n = BUFFER_SIZE - (this->buffer_tail - this->buffer); + if(((this->buffer_get_ptr - this->buffer) + n) > BUFFER_SIZE) { + n = BUFFER_SIZE - (this->buffer_get_ptr - this->buffer); } /* the actual read */ - memcpy(buf, this->buffer_tail, n); + memcpy(buf, this->buffer_get_ptr, n); buf += n; copied += n; length -= n; /* update the tail pointer, watch for wrap arounds */ - this->buffer_tail += n; - if(this->buffer_tail - this->buffer >= BUFFER_SIZE) - this->buffer_tail = this->buffer; + this->buffer_get_ptr += n; + if(this->buffer_get_ptr - this->buffer >= BUFFER_SIZE) + this->buffer_get_ptr = this->buffer; - /* lock the buffer, for updating the count */ - pthread_mutex_lock(&this->buffer_mutex); this->buffer_count -= n; - pthread_mutex_unlock(&this->buffer_mutex); /* signal the writer that there's space in the buffer again */ - pthread_mutex_lock(&this->writer_mut); pthread_cond_signal(&this->writer_cond); - pthread_mutex_unlock(&this->writer_mut); + pthread_mutex_unlock(&this->buffer_ring_mut); } this->curpos += copied; @@ -517,6 +519,27 @@ static off_t rtp_plugin_read (input_plugin_t *this_gen, return copied; } +static buf_element_t *rtp_plugin_read_block (input_plugin_t *this_gen, + fifo_buffer_t *fifo, off_t todo) { + buf_element_t *buf = fifo->buffer_pool_alloc (fifo); + int total_bytes; + + + buf->content = buf->mem; + buf->type = BUF_DEMUX_BLOCK; + + total_bytes = rtp_plugin_read (this_gen, buf->content, todo); + + if (total_bytes != todo) { + buf->free_buffer (buf); + return NULL; + } + + buf->size = total_bytes; + + return buf; +} + /* * */ @@ -584,9 +607,11 @@ static int rtp_plugin_get_optional_data (input_plugin_t *this_gen, */ if (data_type == INPUT_OPTIONAL_DATA_PREVIEW) { - if (this->preview_size == 0) { + if (!this->preview_read_done) { this->preview_size = rtp_plugin_read(this_gen, this->preview, MAX_PREVIEW_SIZE); lprintf("Preview data length = %d\n", this->preview_size); + + this->preview_read_done = 1; } memcpy(data, this->preview, this->preview_size); return this->preview_size; @@ -705,23 +730,21 @@ static input_plugin_t *rtp_class_get_instance (input_class_t *cls_gen, if (iptr) this->interface = iptr; - pthread_mutex_init(&this->buffer_mutex, NULL); - pthread_mutex_init(&this->reader_mut, NULL); - pthread_mutex_init(&this->writer_mut, NULL); + pthread_mutex_init(&this->buffer_ring_mut, NULL); pthread_cond_init(&this->reader_cond, NULL); pthread_cond_init(&this->writer_cond, NULL); this->buffer = malloc(BUFFER_SIZE); - this->buffer_head = this->buffer; - this->buffer_tail = this->buffer; + this->buffer_put_ptr = this->buffer; + this->buffer_get_ptr = this->buffer; this->buffer_count = 0; this->curpos = 0; this->input_plugin.open = rtp_plugin_open; this->input_plugin.get_capabilities = rtp_plugin_get_capabilities; this->input_plugin.read = rtp_plugin_read; - this->input_plugin.read_block = NULL; + this->input_plugin.read_block = rtp_plugin_read_block; this->input_plugin.seek = rtp_plugin_seek; this->input_plugin.get_current_pos = rtp_plugin_get_current_pos; this->input_plugin.get_length = rtp_plugin_get_length; -- cgit v1.2.3 From d7008b1c096a1d537313f95f872808460f9ffada Mon Sep 17 00:00:00 2001 From: Simon Farnsworth Date: Thu, 12 Jul 2007 11:18:15 +0100 Subject: Allow input_dvb to timeout on no signal If there's no signal, the tuner never goes to FE_TIMEDOUT. Add a separate timeout, to prevent xine waiting forever in these situations. --- src/input/input_dvb.c | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/input/input_dvb.c b/src/input/input_dvb.c index 98b343b44..5b3f0b086 100644 --- a/src/input/input_dvb.c +++ b/src/input/input_dvb.c @@ -1001,6 +1001,9 @@ static int tuner_tune_it (tuner_t *this, struct dvb_frontend_parameters struct dvb_frontend_event event; unsigned int strength; struct pollfd pfd[1]; + xine_cfg_entry_t config_tuning_timeout; + struct timeval time_now; + struct timeval tuning_timeout; /* discard stale events */ while (ioctl(this->fd_frontend, FE_GET_EVENT, &event) != -1); @@ -1028,6 +1031,19 @@ static int tuner_tune_it (tuner_t *this, struct dvb_frontend_parameters return 0; } } + + xine_config_lookup_entry(this->xine, "media.dvb.tuning_timeout", &config_tuning_timeout); + xprintf(this->xine, XINE_VERBOSITY_DEBUG, "input_dvb: media.dvb.tuning_timeout is %d\n", config_tuning_timeout.num_value ); + + if( config_tuning_timeout.num_value != 0 ) { + gettimeofday( &tuning_timeout, NULL ); + if( config_tuning_timeout.num_value < 5 ) + tuning_timeout.tv_sec += 5; + else + tuning_timeout.tv_sec += config_tuning_timeout.num_value; + } + + xprintf(this->xine, XINE_VERBOSITY_DEBUG, "input_dvb: tuner_tune_it - waiting for lock...\n" ); do { status = 0; @@ -1040,8 +1056,20 @@ static int tuner_tune_it (tuner_t *this, struct dvb_frontend_parameters if (status & FE_HAS_LOCK) { break; } - usleep(500000); - print_error("Trying to get lock..."); + + /* FE_TIMEDOUT does not happen in a no signal condition. + * Use the tuning_timeout config to prevent a hang in this loop + */ + if( config_tuning_timeout.num_value != 0 ) { + gettimeofday( &time_now, NULL ); + if( time_now.tv_sec > tuning_timeout.tv_sec ) { + xprintf(this->xine, XINE_VERBOSITY_DEBUG, "input_dvb: No FE_HAS_LOCK before timeout\n"); + break; + } + } + + usleep(10000); + xprintf(this->xine, XINE_VERBOSITY_DEBUG, "Trying to get lock..."); } while (!(status & FE_TIMEDOUT)); /* inform the user of frontend status */ @@ -3250,6 +3278,12 @@ static void *init_class (xine_t *xine, void *data) { _("If enabled xine will remember and switch to this channel. "), 21, NULL, NULL); + config->register_num(config, "media.dvb.tuning_timeout", + 0, + _("Number of seconds until tuning times out."), + _("Leave at 0 means try forever. " + "Greater then 0 means wait that many seconds to get a lock. Minimum is 5 seconds."), + 0, NULL, (void *) this); config->register_num(config, "media.dvb.adapter", 0, -- cgit v1.2.3 From 11a62668b3a6e83574470ada57ed150bcee66d88 Mon Sep 17 00:00:00 2001 From: Darren Salt Date: Fri, 13 Jul 2007 20:32:38 +0100 Subject: Fix a spelling error in the media.dvb.tuning_timeout description. --- src/input/input_dvb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/input/input_dvb.c b/src/input/input_dvb.c index 5b3f0b086..b3a399ce4 100644 --- a/src/input/input_dvb.c +++ b/src/input/input_dvb.c @@ -3282,7 +3282,7 @@ static void *init_class (xine_t *xine, void *data) { 0, _("Number of seconds until tuning times out."), _("Leave at 0 means try forever. " - "Greater then 0 means wait that many seconds to get a lock. Minimum is 5 seconds."), + "Greater than 0 means wait that many seconds to get a lock. Minimum is 5 seconds."), 0, NULL, (void *) this); config->register_num(config, "media.dvb.adapter", -- cgit v1.2.3