From 4e268aae55c483ec6c743f741f17a8a0f9250f1e Mon Sep 17 00:00:00 2001 From: Marton Balint Date: Sat, 8 Oct 2011 21:38:53 +0200 Subject: [PATCH 1/4] ffplay: calculate target clock dynamically, make code more readable Since target clock is based on the current A-V delay, it is better calculate it when we actually need it rather than when we put a picture in the picture queue. The patch also makes a code a bit more readable by renaming some delay variables to duration, and converting compute_target_time to a delay calculating function which does not modify the state. Factoring out the iteration of the pictq to standalone function is also done in this patch. Signed-off-by: Marton Balint --- ffplay.c | 97 +++++++++++++++++++++++++++++--------------------------- 1 file changed, 50 insertions(+), 47 deletions(-) diff --git a/ffplay.c b/ffplay.c index 2bf5d27acf..efdbe58bbd 100644 --- a/ffplay.c +++ b/ffplay.c @@ -98,9 +98,9 @@ typedef struct PacketQueue { typedef struct VideoPicture { double pts; ///paused = !is->paused; } -static double compute_target_time(double frame_current_pts, VideoState *is) +static double compute_target_delay(double delay, VideoState *is) { - double delay, sync_threshold, diff; - - /* compute nominal delay */ - delay = frame_current_pts - is->frame_last_pts; - if (delay <= 0 || delay >= 10.0) { - /* if incorrect delay, use previous one */ - delay = is->frame_last_delay; - } else { - is->frame_last_delay = delay; - } - is->frame_last_pts = frame_current_pts; + double sync_threshold, diff; /* update delay to follow master synchronisation source */ if (((is->av_sync_type == AV_SYNC_AUDIO_MASTER && is->audio_st) || @@ -1103,12 +1093,22 @@ static double compute_target_time(double frame_current_pts, VideoState *is) delay = 2 * delay; } } - is->frame_timer += delay; - av_dlog(NULL, "video: delay=%0.3f pts=%0.3f A-V=%f\n", - delay, frame_current_pts, -diff); + av_dlog(NULL, "video: delay=%0.3f A-V=%f\n", + delay, -diff); - return is->frame_timer; + return delay; +} + +static void pictq_next_picture(VideoState *is) { + /* update queue size and signal for next picture */ + if (++is->pictq_rindex == VIDEO_PICTURE_QUEUE_SIZE) + is->pictq_rindex = 0; + + SDL_LockMutex(is->pictq_mutex); + is->pictq_size--; + SDL_CondSignal(is->pictq_cond); + SDL_UnlockMutex(is->pictq_mutex); } /* called to display each frame */ @@ -1125,34 +1125,45 @@ retry: //nothing to do, no picture to display in the que } else { double time= av_gettime()/1000000.0; - double next_target; + double last_duration, duration, delay; /* dequeue the picture */ vp = &is->pictq[is->pictq_rindex]; - if(time < vp->target_clock) + if (vp->skip) { + pictq_next_picture(is); + goto retry; + } + + /* compute nominal last_duration */ + last_duration = vp->pts - is->frame_last_pts; + if (last_duration > 0 && last_duration < 10.0) { + /* if duration of the last frame was sane, update last_duration in video state */ + is->frame_last_duration = last_duration; + } + delay = compute_target_delay(is->frame_last_duration, is); + + if(time < is->frame_timer + delay) return; + + is->frame_last_pts = vp->pts; + is->frame_timer += delay; + /* update current video pts */ is->video_current_pts = vp->pts; is->video_current_pts_drift = is->video_current_pts - time; is->video_current_pos = vp->pos; - if(is->pictq_size > 1){ - VideoPicture *nextvp= &is->pictq[(is->pictq_rindex+1)%VIDEO_PICTURE_QUEUE_SIZE]; - assert(nextvp->target_clock >= vp->target_clock); - next_target= nextvp->target_clock; - }else{ - next_target= vp->target_clock + vp->duration; + + if(is->pictq_size > 1) { + VideoPicture *nextvp= &is->pictq[(is->pictq_rindex+1)%VIDEO_PICTURE_QUEUE_SIZE]; + duration = nextvp->pts - vp->pts; // More accurate this way, 1/time_base is often not reflecting FPS + } else { + duration = vp->duration; } - if((framedrop>0 || (framedrop && is->audio_st)) && time > next_target){ + + if((framedrop>0 || (framedrop && is->audio_st)) && time > is->frame_timer + duration){ is->skip_frames *= 1.0 + FRAME_SKIP_FACTOR; if(is->pictq_size > 1){ - /* update queue size and signal for next picture */ - if (++is->pictq_rindex == VIDEO_PICTURE_QUEUE_SIZE) - is->pictq_rindex = 0; - - SDL_LockMutex(is->pictq_mutex); - is->pictq_size--; - SDL_CondSignal(is->pictq_cond); - SDL_UnlockMutex(is->pictq_mutex); + pictq_next_picture(is); goto retry; } } @@ -1205,14 +1216,7 @@ retry: if (!display_disable) video_display(is); - /* update queue size and signal for next picture */ - if (++is->pictq_rindex == VIDEO_PICTURE_QUEUE_SIZE) - is->pictq_rindex = 0; - - SDL_LockMutex(is->pictq_mutex); - is->pictq_size--; - SDL_CondSignal(is->pictq_cond); - SDL_UnlockMutex(is->pictq_mutex); + pictq_next_picture(is); } } else if (is->audio_st) { /* draw the next audio frame */ @@ -1425,13 +1429,12 @@ static int queue_picture(VideoState *is, AVFrame *src_frame, double pts1, int64_ vp->pts = pts; vp->pos = pos; + vp->skip = 0; /* now we can update the picture count */ if (++is->pictq_windex == VIDEO_PICTURE_QUEUE_SIZE) is->pictq_windex = 0; SDL_LockMutex(is->pictq_mutex); - vp->target_clock= compute_target_time(vp->pts, is); - is->pictq_size++; SDL_UnlockMutex(is->pictq_mutex); } @@ -1451,7 +1454,7 @@ static int get_video_frame(VideoState *is, AVFrame *frame, int64_t *pts, AVPacke SDL_LockMutex(is->pictq_mutex); //Make sure there are no long delay timers (ideally we should just flush the que but thats harder) for (i = 0; i < VIDEO_PICTURE_QUEUE_SIZE; i++) { - is->pictq[i].target_clock= 0; + is->pictq[i].skip = 1; } while (is->pictq_size && !is->videoq.abort_request) { SDL_CondWait(is->pictq_cond, is->pictq_mutex); @@ -1460,7 +1463,7 @@ static int get_video_frame(VideoState *is, AVFrame *frame, int64_t *pts, AVPacke SDL_UnlockMutex(is->pictq_mutex); is->frame_last_pts = AV_NOPTS_VALUE; - is->frame_last_delay = 0; + is->frame_last_duration = 0; is->frame_timer = (double)av_gettime() / 1000000.0; is->skip_frames = 1; is->skip_frames_index = 0; From d2d8e1e599a4b5c01e5be9267ac85105e843d928 Mon Sep 17 00:00:00 2001 From: Marton Balint Date: Sun, 9 Oct 2011 15:49:22 +0200 Subject: [PATCH 2/4] ffplay: remove early frame drop functionality The current impementation of early frame drops (dropping frames before adding them to the picture queue) has multiple problems: Even after gettin A-V sync, the frame droping continues until VideoState->skip_frames reaches 1, which can take a lot of time causing useless additional frame drops and bad AV-sync. This issue can be easily triggered with for example changing the audio stream. Also video_refresh currenly does not handle early skipped frames in every case, for example if we skip a frame, then the last frame duration calculation will compute the duration of the sum of the skipped frame and the duration of the frame before that, and in compute_target_delay we may multiply this unusually big delay. Signed-off-by: Marton Balint --- ffplay.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/ffplay.c b/ffplay.c index efdbe58bbd..58ed5b8788 100644 --- a/ffplay.c +++ b/ffplay.c @@ -71,8 +71,6 @@ const int program_birth_year = 2003; /* no AV correction is done if too big error */ #define AV_NOSYNC_THRESHOLD 10.0 -#define FRAME_SKIP_FACTOR 0.05 - /* maximum audio speed change to get correct sync */ #define SAMPLE_CORRECTION_PERCENT_MAX 10 @@ -221,8 +219,6 @@ typedef struct VideoState { AVFilterContext *out_video_filter; ///0 || (framedrop && is->audio_st)) && time > is->frame_timer + duration){ - is->skip_frames *= 1.0 + FRAME_SKIP_FACTOR; if(is->pictq_size > 1){ pictq_next_picture(is); goto retry; @@ -1248,10 +1243,9 @@ retry: av_diff = 0; if (is->audio_st && is->video_st) av_diff = get_audio_clock(is) - get_video_clock(is); - printf("%7.2f A-V:%7.3f s:%3.1f aq=%5dKB vq=%5dKB sq=%5dB f=%"PRId64"/%"PRId64" \r", + printf("%7.2f A-V:%7.3f aq=%5dKB vq=%5dKB sq=%5dB f=%"PRId64"/%"PRId64" \r", get_master_clock(is), av_diff, - FFMAX(is->skip_frames-1, 0), aqsize / 1024, vqsize / 1024, sqsize, @@ -1335,9 +1329,6 @@ static int queue_picture(VideoState *is, AVFrame *src_frame, double pts1, int64_ /* wait until we have space to put a new picture */ SDL_LockMutex(is->pictq_mutex); - if(is->pictq_size>=VIDEO_PICTURE_QUEUE_SIZE && !is->refresh) - is->skip_frames= FFMAX(1.0 - FRAME_SKIP_FACTOR, is->skip_frames * (1.0-FRAME_SKIP_FACTOR)); - while (is->pictq_size >= VIDEO_PICTURE_QUEUE_SIZE && !is->videoq.abort_request) { SDL_CondWait(is->pictq_cond, is->pictq_mutex); @@ -1465,8 +1456,6 @@ static int get_video_frame(VideoState *is, AVFrame *frame, int64_t *pts, AVPacke is->frame_last_pts = AV_NOPTS_VALUE; is->frame_last_duration = 0; is->frame_timer = (double)av_gettime() / 1000000.0; - is->skip_frames = 1; - is->skip_frames_index = 0; return 0; } @@ -1485,11 +1474,7 @@ static int get_video_frame(VideoState *is, AVFrame *frame, int64_t *pts, AVPacke *pts = 0; } - is->skip_frames_index += 1; - if(is->skip_frames_index >= is->skip_frames){ - is->skip_frames_index -= FFMAX(is->skip_frames, 1.0); - return 1; - } + return 1; } return 0; From abb0e4f63772fe96f5e221b8a579180cb4ebe5a9 Mon Sep 17 00:00:00 2001 From: Marton Balint Date: Sun, 9 Oct 2011 16:52:28 +0200 Subject: [PATCH 3/4] ffplay: add delay multiple times to frame_timer if it is less than current time If the picture queue is empty, or when the calculated delay is 0, frame_timer is not increased but we are still displaying the old frame. When we eventually get a frame, where the computed delay is positive, so we don't need to drop any more frames, then it is best to update frame_timer to be as near as the current time as it can. This way we dont't have to wait several frames to add the necesarry delays to frame_timer to reach current time, therefore there are no extra frame drops after reaching a positive delay. Signed-off-by: Marton Balint --- ffplay.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ffplay.c b/ffplay.c index 58ed5b8788..a0c8487637 100644 --- a/ffplay.c +++ b/ffplay.c @@ -1142,7 +1142,8 @@ retry: return; is->frame_last_pts = vp->pts; - is->frame_timer += delay; + if (delay > 0) + is->frame_timer += delay * FFMAX(1, floor((time-is->frame_timer) / delay)); /* update current video pts */ is->video_current_pts = vp->pts; From 72776adfb9f0b856198a770acdec97d523c01208 Mon Sep 17 00:00:00 2001 From: Jean First Date: Wed, 12 Oct 2011 21:54:33 +0200 Subject: [PATCH 4/4] ffplay: avoid window resize crash on osx with libsdl 1.2.14 Signed-off-by: Marton Balint --- ffplay.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/ffplay.c b/ffplay.c index a0c8487637..b41820c51b 100644 --- a/ffplay.c +++ b/ffplay.c @@ -943,13 +943,7 @@ static int video_open(VideoState *is){ if(screen && is->width == screen->w && screen->w == w && is->height== screen->h && screen->h == h) return 0; - -#ifndef __APPLE__ screen = SDL_SetVideoMode(w, h, 0, flags); -#else - /* setting bits_per_pixel = 0 or 32 causes blank video on OS X */ - screen = SDL_SetVideoMode(w, h, 24, flags); -#endif if (!screen) { fprintf(stderr, "SDL: could not set video mode - exiting\n"); do_exit(is);