From b4304f243c6006eec3ca4b4ce2b19cfca703861a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reinhard=20Ni=C3=9Fl?= Date: Thu, 12 Apr 2007 22:14:06 +0200 Subject: Extend ticket system for nonblocking ticket acquiries. The current code has a race condition which can block arbitrary threads that call for example xine_get_current_frame() until the stream gets unpaused again. This can happen when the internal ticket acquiration collides with a ticket revokation for example when another thread is going to pause the stream. There are a few situations where a port ticket needs to be acquired for calling a port function but where it is absolutely undesireable to get blocked for an undetermined period of time. Therefore the ticket system should be extended by nonblocking functions which allow ticket acquiration even when a ticket revokation is in progress. And in the case where blocking is not avoidable, it should simply be indicated that no ticket was acquired. The caller can then choose to repeat the call at a later point in time. --- src/xine-engine/xine.c | 49 ++++++++++++++++++++++++++++++++--------- src/xine-engine/xine_internal.h | 9 ++++++++ 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/src/xine-engine/xine.c b/src/xine-engine/xine.c index f49a988c9..612bf8dcc 100644 --- a/src/xine-engine/xine.c +++ b/src/xine-engine/xine.c @@ -127,23 +127,42 @@ void _x_extra_info_merge( extra_info_t *dst, extra_info_t *src ) { } } -static void ticket_acquire(xine_ticket_t *this, int irrevocable) { +static int ticket_acquire_internal(xine_ticket_t *this, int irrevocable, int nonblocking) { + int must_wait = 0; pthread_mutex_lock(&this->lock); if (this->ticket_revoked && !this->irrevocable_tickets) - pthread_cond_wait(&this->issued, &this->lock); + must_wait = !nonblocking; else if (this->atomic_revoke && !pthread_equal(this->atomic_revoker_thread, pthread_self())) + must_wait = 1; + + if (must_wait) { + if (nonblocking) { + pthread_mutex_unlock(&this->lock); + return 0; + } + pthread_cond_wait(&this->issued, &this->lock); + } this->tickets_granted++; if (irrevocable) this->irrevocable_tickets++; pthread_mutex_unlock(&this->lock); + return 1; } -static void ticket_release(xine_ticket_t *this, int irrevocable) { +static int ticket_acquire_nonblocking(xine_ticket_t *this, int irrevocable) { + return ticket_acquire_internal(this, irrevocable, 1); +} + +static void ticket_acquire(xine_ticket_t *this, int irrevocable) { + ticket_acquire_internal(this, irrevocable, 0); +} + +static void ticket_release_internal(xine_ticket_t *this, int irrevocable, int nonblocking) { pthread_mutex_lock(&this->lock); @@ -153,12 +172,20 @@ static void ticket_release(xine_ticket_t *this, int irrevocable) { if (this->ticket_revoked && !this->tickets_granted) pthread_cond_broadcast(&this->revoked); - if (this->ticket_revoked && !this->irrevocable_tickets) + if (this->ticket_revoked && !this->irrevocable_tickets && !nonblocking) pthread_cond_wait(&this->issued, &this->lock); pthread_mutex_unlock(&this->lock); } +static void ticket_release_nonblocking(xine_ticket_t *this, int irrevocable) { + ticket_release_internal(this, irrevocable, 1); +} + +static void ticket_release(xine_ticket_t *this, int irrevocable) { + ticket_release_internal(this, irrevocable, 0); +} + static void ticket_renew(xine_ticket_t *this, int irrevocable) { pthread_mutex_lock(&this->lock); @@ -227,12 +254,14 @@ static xine_ticket_t *ticket_init(void) { port_ticket = (xine_ticket_t *) xine_xmalloc(sizeof(xine_ticket_t)); - port_ticket->acquire = ticket_acquire; - port_ticket->release = ticket_release; - port_ticket->renew = ticket_renew; - port_ticket->issue = ticket_issue; - port_ticket->revoke = ticket_revoke; - port_ticket->dispose = ticket_dispose; + port_ticket->acquire_nonblocking = ticket_acquire_nonblocking; + port_ticket->acquire = ticket_acquire; + port_ticket->release_nonblocking = ticket_release_nonblocking; + port_ticket->release = ticket_release; + port_ticket->renew = ticket_renew; + port_ticket->issue = ticket_issue; + port_ticket->revoke = ticket_revoke; + port_ticket->dispose = ticket_dispose; 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 30899a4b3..da6f88a7f 100644 --- a/src/xine-engine/xine_internal.h +++ b/src/xine-engine/xine_internal.h @@ -159,6 +159,15 @@ struct xine_ticket_s { * revocation or by other threads acquiring tickets */ void (*revoke)(xine_ticket_t *self, int atomic); + /* behaves like acquire() but doesn't block the calling thread; when + * the thread would have been blocked, 0 is returned otherwise 1 + * this function acquires a ticket even if ticket revocation is active */ + int (*acquire_nonblocking)(xine_ticket_t *self, int irrevocable); + + /* behaves like release() but doesn't block the calling thread; should + * be used in combination with acquire_nonblocking() */ + void (*release_nonblocking)(xine_ticket_t *self, int irrevocable); + void (*dispose)(xine_ticket_t *self); pthread_mutex_t lock; -- cgit v1.2.3 From bd88a5c94af0af727680606a22ec9414fba68366 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reinhard=20Ni=C3=9Fl?= Date: Thu, 12 Apr 2007 22:33:26 +0200 Subject: Provide a function to query buffer usage. This function shall be used to poll the number of remaining frames from a certain point in time on until the reported numbers are all 0. At that point in time, the content on screen is identical to a certain state of the stream, at which for example, a hardcopy may be taken. --- src/xine-engine/xine.c | 28 ++++++++++++++++++++++++++++ src/xine-engine/xine_internal.h | 2 ++ 2 files changed, 30 insertions(+) diff --git a/src/xine-engine/xine.c b/src/xine-engine/xine.c index 612bf8dcc..9623668dc 100644 --- a/src/xine-engine/xine.c +++ b/src/xine-engine/xine.c @@ -2069,3 +2069,31 @@ int xine_stream_master_slave(xine_stream_t *master, xine_stream_t *slave, slave->master = master->master; return 1; } + +int _x_query_buffer_usage(xine_stream_t *stream, int *num_video_buffers, int *num_audio_buffers, int *num_video_frames, int *num_audio_frames) +{ + int ticket_acquired = -1; + + if (num_video_buffers) + *num_video_buffers = (stream->video_fifo ? stream->video_fifo->size(stream->video_fifo) : 0); + + if (num_audio_buffers) + *num_audio_buffers = (stream->audio_fifo ? stream->audio_fifo->size(stream->audio_fifo) : 0); + + if ((num_video_frames && stream->video_out) + || (num_audio_frames && stream->audio_out)) { + + ticket_acquired = stream->xine->port_ticket->acquire_nonblocking(stream->xine->port_ticket, 1); + } + + if (num_video_frames) + *num_video_frames = ((ticket_acquired && stream->video_out) ? stream->video_out->get_property(stream->video_out, VO_PROP_BUFS_IN_FIFO) : 0); + + if (num_audio_frames) + *num_audio_frames = ((ticket_acquired && stream->audio_out) ? stream->audio_out->get_property(stream->audio_out, AO_PROP_BUFS_IN_FIFO) : 0); + + if (ticket_acquired > 0) + stream->xine->port_ticket->release_nonblocking(stream->xine->port_ticket, 1); + + return ticket_acquired != 0; +} diff --git a/src/xine-engine/xine_internal.h b/src/xine-engine/xine_internal.h index da6f88a7f..c88bcc904 100644 --- a/src/xine-engine/xine_internal.h +++ b/src/xine-engine/xine_internal.h @@ -366,6 +366,8 @@ struct xine_stream_s { * private function prototypes: */ +int _x_query_buffer_usage(xine_stream_t *stream, int *num_video_buffers, int *num_audio_buffers, int *num_video_frames, int *num_audio_frames) XINE_PROTECTED; + void _x_handle_stream_end (xine_stream_t *stream, int non_user) XINE_PROTECTED; /* report message to UI. usually these are async errors */ -- cgit v1.2.3 From f01ea9ec4d506bf4080a569129cafe4a966b0724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reinhard=20Ni=C3=9Fl?= Date: Thu, 12 Apr 2007 22:59:54 +0200 Subject: Let xxmc switch back to an unaccelerated context. The current code misses the ability to switch back to an unaccelerated context, e. g. when previously MPEG2 material was displayed which is then followed by H.264 material. As the latter is not handled as XINE_IMGFMT_XXMC there was no way to leave the accelerated context and therefore the images did not appear on screen. --- src/video_out/video_out_xxmc.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/video_out/video_out_xxmc.c b/src/video_out/video_out_xxmc.c index fd19f391b..498eaab33 100644 --- a/src/video_out/video_out_xxmc.c +++ b/src/video_out/video_out_xxmc.c @@ -977,7 +977,7 @@ static void xvmc_check_colorkey_properties(xxmc_driver_t *driver) static int xxmc_xvmc_update_context(xxmc_driver_t *driver, xxmc_frame_t *frame, - uint32_t width, uint32_t height) + uint32_t width, uint32_t height, int frame_format_xxmc) { xine_xxmc_t *xxmc = &frame->xxmc_data; @@ -991,8 +991,12 @@ static int xxmc_xvmc_update_context(xxmc_driver_t *driver, xxmc_frame_t *frame, xprintf(driver->xine, XINE_VERBOSITY_LOG, "video_out_xxmc: New format. Need to change XvMC Context.\n" - "width: %d height: %d mpeg: %d acceleration: %d\n", width, height, - xxmc->mpeg, xxmc->acceleration); + "width: %d height: %d", width, height); + if (frame_format_xxmc) { + xprintf(driver->xine, XINE_VERBOSITY_LOG, + " mpeg: %d acceleration: %d", xxmc->mpeg, xxmc->acceleration); + } + xprintf(driver->xine, XINE_VERBOSITY_LOG, "\n"); if (frame->xvmc_surf) xxmc_xvmc_free_surface( driver , frame->xvmc_surf); @@ -1000,7 +1004,7 @@ static int xxmc_xvmc_update_context(xxmc_driver_t *driver, xxmc_frame_t *frame, xxmc_dispose_context( driver ); - if (xxmc_find_context( driver, xxmc, width, height )) { + if (frame_format_xxmc && xxmc_find_context( driver, xxmc, width, height )) { xxmc_create_context( driver, width, height); xvmc_check_colorkey_properties( driver ); xxmc_setup_subpictures(driver, width, height); @@ -1231,7 +1235,7 @@ static void xxmc_do_update_frame(vo_driver_t *this_gen, (this->xvmc_width != width) || (this->xvmc_height != height)) { this->last_accel_request = xxmc->acceleration; - xxmc_xvmc_update_context(this, frame, width, height); + xxmc_xvmc_update_context(this, frame, width, height, 1); } else { this->last_accel_request = xxmc->acceleration; } @@ -1254,6 +1258,11 @@ static void xxmc_do_update_frame(vo_driver_t *this_gen, xvmc_context_writer_unlock( &this->xvmc_lock); } else { + /* switch back to an unaccelerated context */ + if (this->last_accel_request != 0xFFFFFFFF) { + this->last_accel_request = 0xFFFFFFFF; + xxmc_xvmc_update_context(this, frame, width, height, 0); + } frame->vo_frame.proc_duplicate_frame_data = NULL; xxmc_do_update_frame_xv(this_gen, frame_gen, width, height, ratio, format, flags); -- cgit v1.2.3 From e8aa9780585fcec85b2a019af6e248687189e97c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reinhard=20Ni=C3=9Fl?= Date: Thu, 12 Apr 2007 23:12:28 +0200 Subject: Avoid keeping frames referenced which are nolonger used. The current implementation keeps references to VO_NUM_RECENT_FRAMES frames (for deinterlacing), but doesn't make any use of them. As many XXMC capable devices only supply 8 frames at all, keeping fewer frames referenced makes more available for decoding and thus avoids frame drops by keeping the number of frames which are ready for display more often above frame_drop_limit. --- src/video_out/video_out_xxmc.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/video_out/video_out_xxmc.c b/src/video_out/video_out_xxmc.c index 498eaab33..53e4b7995 100644 --- a/src/video_out/video_out_xxmc.c +++ b/src/video_out/video_out_xxmc.c @@ -1598,6 +1598,20 @@ static void xxmc_display_frame (vo_driver_t *this_gen, vo_frame_t *frame_gen) xxmc_add_recent_frame (this, frame); /* deinterlacing */ + /* + * the current implementation doesn't need recent frames for deinterlacing, + * but as most of the time we only have a little number of frames available + * per device, we only hold references to the most recent frame by filling + * the whole buffer with the same frame + */ + { + int i; + for (i = 1; i < VO_NUM_RECENT_FRAMES; i++) { + frame->vo_frame.lock(&frame->vo_frame); + xxmc_add_recent_frame (this, frame); /* deinterlacing */ + } + } + if ((frame->format == XINE_IMGFMT_XXMC) && (!xxmc->decoded || !xxmc_xvmc_surface_valid(this, frame->xvmc_surf))) { xvmc_context_reader_unlock( &this->xvmc_lock ); @@ -1943,7 +1957,7 @@ static void xxmc_dispose (vo_driver_t *this_gen) { for( i=0; i < VO_NUM_RECENT_FRAMES; i++ ) { if( this->recent_frames[i] ) - this->recent_frames[i]->vo_frame.dispose + this->recent_frames[i]->vo_frame.free (&this->recent_frames[i]->vo_frame); this->recent_frames[i] = NULL; } -- cgit v1.2.3 From 7b50cbef25c8e2769bfbdd801b7318bb86047f87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reinhard=20Ni=C3=9Fl?= Date: Thu, 12 Apr 2007 23:36:51 +0200 Subject: Make bob deinterlacing more precisely and skip it on demand. Bob deinterlacing is implemented as showing the top field, sleeping for half the frame duration and showing the bottom field. Most drivers tend to synchronize displaying a field on the VBI and thus displaying a field may take up to half the frame duration in certain cases. According to the original code, the sleep took always half the frame duration and therefore the second field could get displayed too late. As a result, the driver was syncing to VBI most often, so that things got even worse. The changed code now calculates the sleep time in a way that the second field gets displayed half the frame duration after the first field. Moreover, it monitors how much time was spent to display the first field and when this time exceeds 75 % of the field time (= half the frame time), it skips displaying the second field, as usually this is an indicator that the driver has no more frame buffers left. So displaying the second field would just make things go worse. --- src/video_out/video_out_xxmc.c | 53 +++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/src/video_out/video_out_xxmc.c b/src/video_out/video_out_xxmc.c index 53e4b7995..37dad7a99 100644 --- a/src/video_out/video_out_xxmc.c +++ b/src/video_out/video_out_xxmc.c @@ -1588,6 +1588,12 @@ static void xxmc_display_frame (vo_driver_t *this_gen, vo_frame_t *frame_gen) xxmc_frame_t *frame = (xxmc_frame_t *) frame_gen; xine_xxmc_t *xxmc = &frame->xxmc_data; int first_field; + struct timeval tv_top; + + /* + * take time to calculate the time to sleep for the bottom field + */ + gettimeofday(&tv_top, 0); /* * queue frames (deinterlacing) @@ -1653,20 +1659,41 @@ static void xxmc_display_frame (vo_driver_t *this_gen, vo_frame_t *frame_gen) this->cur_field); XVMCUNLOCKDISPLAY( this->display ); if (this->deinterlace_enabled && this->bob) { - unsigned - ms_per_field = 500 * frame->vo_frame.duration / 90000 - 2; - - usleep(ms_per_field*1000); - this->cur_field = (frame->vo_frame.top_field_first) ? XVMC_BOTTOM_FIELD : XVMC_TOP_FIELD; + struct timeval tv_middle; + long us_spent_so_far, us_per_field = frame->vo_frame.duration * 50 / 9; - XVMCLOCKDISPLAY( this->display ); - XvMCPutSurface( this->display, frame->xvmc_surf , this->drawable, - this->sc.displayed_xoffset, this->sc.displayed_yoffset, - this->sc.displayed_width, this->sc.displayed_height, - this->sc.output_xoffset, this->sc.output_yoffset, - this->sc.output_width, this->sc.output_height, - this->cur_field); - XVMCUNLOCKDISPLAY( this->display ); + gettimeofday(&tv_middle, 0); + us_spent_so_far = (tv_middle.tv_sec - tv_top.tv_sec) * 1000000 + (tv_middle.tv_usec - tv_top.tv_usec); + if (us_spent_so_far < 0) + us_spent_so_far = 0; + + /* + * typically, the operations above take just a few milliseconds, but when the + * driver actively waits to sync on the next field, we better skip showing the + * other field as it would lead to further busy waiting + * so display the other field only if we've spent less than 75 % of the per + * field time so far + */ + if (4 * us_spent_so_far < 3 * us_per_field) { + long us_delay = (us_per_field - 2000) - us_spent_so_far; + if (us_delay > 0) { + xvmc_context_reader_unlock( &this->xvmc_lock ); + xine_usec_sleep(us_delay); + LOCK_AND_SURFACE_VALID( this, frame->xvmc_surf ); + } + + this->cur_field = (frame->vo_frame.top_field_first) ? XVMC_BOTTOM_FIELD : XVMC_TOP_FIELD; + + XVMCLOCKDISPLAY( this->display ); + XvMCPutSurface( this->display, frame->xvmc_surf , this->drawable, + this->sc.displayed_xoffset, this->sc.displayed_yoffset, + this->sc.displayed_width, this->sc.displayed_height, + this->sc.output_xoffset, this->sc.output_yoffset, + this->sc.output_width, this->sc.output_height, + this->cur_field); + + XVMCUNLOCKDISPLAY( this->display ); + } } } else { XLockDisplay (this->display); -- cgit v1.2.3 From 60871f034457fb6727df480c311b76e81a4ca6fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reinhard=20Ni=C3=9Fl?= Date: Thu, 12 Apr 2007 23:51:04 +0200 Subject: Disable bob deinterlacing on a per frame decision. There are some situations where bob deinterlacing doesn't make much sense, as the effect doesn't work for example for slow motion. And in fast motion mode, the sleep between displaying the two fields lowers the reachable fast motion frame rate. Another annoying effect is the reduction in vertical resolution for still images. The changed code tries to detect such issues and disables bob deinterlacing for such frames. The detection of still images is still to come. --- src/video_out/video_out_xxmc.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/video_out/video_out_xxmc.c b/src/video_out/video_out_xxmc.c index 37dad7a99..20ce00a5e 100644 --- a/src/video_out/video_out_xxmc.c +++ b/src/video_out/video_out_xxmc.c @@ -1588,6 +1588,7 @@ static void xxmc_display_frame (vo_driver_t *this_gen, vo_frame_t *frame_gen) xxmc_frame_t *frame = (xxmc_frame_t *) frame_gen; xine_xxmc_t *xxmc = &frame->xxmc_data; int first_field; + int disable_deinterlace = 0; struct timeval tv_top; /* @@ -1595,6 +1596,19 @@ static void xxmc_display_frame (vo_driver_t *this_gen, vo_frame_t *frame_gen) */ gettimeofday(&tv_top, 0); + /* + * bob deinterlacing doesn't make much sense for still images or at replay speeds + * other than 100 %, so let's disable deinterlacing at all for this frame + */ + if (this->deinterlace_enabled && this->bob) { + disable_deinterlace = frame->vo_frame.progressive_frame + || !frame->vo_frame.stream + || xine_get_param(frame->vo_frame.stream, XINE_PARAM_FINE_SPEED) != XINE_FINE_SPEED_NORMAL; + if (!disable_deinterlace) { + /* TODO: still frame detection */ + } + } + /* * queue frames (deinterlacing) * free old frames @@ -1645,7 +1659,7 @@ static void xxmc_display_frame (vo_driver_t *this_gen, vo_frame_t *frame_gen) first_field = (frame->vo_frame.top_field_first) ? XVMC_TOP_FIELD : XVMC_BOTTOM_FIELD; first_field = (this->bob) ? first_field : XVMC_TOP_FIELD; - this->cur_field = (this->deinterlace_enabled) ? first_field : XVMC_FRAME_PICTURE; + this->cur_field = (this->deinterlace_enabled && !disable_deinterlace) ? first_field : XVMC_FRAME_PICTURE; xxmc_redraw_needed (this_gen); if (frame->format == XINE_IMGFMT_XXMC) { @@ -1658,7 +1672,7 @@ static void xxmc_display_frame (vo_driver_t *this_gen, vo_frame_t *frame_gen) this->sc.output_width, this->sc.output_height, this->cur_field); XVMCUNLOCKDISPLAY( this->display ); - if (this->deinterlace_enabled && this->bob) { + if (this->deinterlace_enabled && !disable_deinterlace && this->bob) { struct timeval tv_middle; long us_spent_so_far, us_per_field = frame->vo_frame.duration * 50 / 9; -- cgit v1.2.3 From 42db183b596601354643b6e8f4997109b2324f6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reinhard=20Ni=C3=9Fl?= Date: Fri, 13 Apr 2007 00:01:57 +0200 Subject: Disable bob deinterlacing on a per frame decision (continued). One can assume that a still frame is to show when there are no more frames left to display. The changed code uses _x_query_buffer_usage() to retrieve the number of frames waiting to be displayed to integrate this information into the decision. --- src/video_out/video_out_xxmc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/video_out/video_out_xxmc.c b/src/video_out/video_out_xxmc.c index 20ce00a5e..11bfc7e1a 100644 --- a/src/video_out/video_out_xxmc.c +++ b/src/video_out/video_out_xxmc.c @@ -1605,7 +1605,9 @@ static void xxmc_display_frame (vo_driver_t *this_gen, vo_frame_t *frame_gen) || !frame->vo_frame.stream || xine_get_param(frame->vo_frame.stream, XINE_PARAM_FINE_SPEED) != XINE_FINE_SPEED_NORMAL; if (!disable_deinterlace) { - /* TODO: still frame detection */ + int vo_bufs_in_fifo = 0; + _x_query_buffer_usage(frame->vo_frame.stream, NULL, NULL, &vo_bufs_in_fifo, NULL); + disable_deinterlace = (vo_bufs_in_fifo <= 0); } } -- cgit v1.2.3