[MPlayer-dev-eng] [PATCH] edl

Michael Halcrow mah69 at email.byu.edu
Fri Jan 17 15:37:45 CET 2003


Hi K,

Thanks for adding EDL to mencoder. However, I would recommend against
the patch in its current form for the following reasons:

1) I already have many optimizations done and waiting to be included.
A'rpi has announced a feature freeze until the 0.90 is released, and I'm
waiting for that to happen before making non-critical changes.

2)

-struct edl_record edl_records[ MAX_EDL_ENTRIES ];

+        actual = (edl_record_ptr)malloc(sizeof(struct edl_record));

 #ifdef USE_EDL
- if( next_edl_record->next ) { // Are we (still?) doing EDL?
-   if( sh_video->pts >= next_edl_record->start_sec ) {

+    float original=actual;
+    while ((edl = get_edl_entry(actual)) != NULL) switch (edl->action)
{
+        case EDL_SKIP:

+edl_record_ptr get_edl_entry(float actual_time)
+{
+ edl_record_ptr fut;
+
+ for (fut=edl_list; fut; fut=fut->next) {
+    if (fut->start_sec <= actual_time && fut->stop_sec > actual_time) {

You've just massively increased the cost-per-cycle for finding the next
EDL entry. Instead of simply checking for a null pointer, it now makes
an entire function call (with parameter passing, activation record
creation, PC jump, and the whole nine yards), and then it seeks through
the *entire* EDL linked list from start to finish. This happens on
*every* cycle during playback. This is inefficient.

If everyone thinks this is a better design for some reason
(unpredictability in how events can affect finding the next EDL record),
then at least implement finding the next record as a binary search:

+	  int mid, left, right;
+	  left = 0;
+	  right = num_edl_records - 1;
+	  mid = ( right - left ) / 2;
+	  while( ( right - left ) > 0 ) {
+	    if( d_video->pts >= edl_records[ mid ].start_sec ) { // We're too
far ahead
+	      left = mid+1;
+	    } else { // We're too far behind
+	      right = mid;
 	    }
+	    mid = ( ( right - left ) / 2 ) + left;
 	  }
+	  next_edl_record = &edl_records[ mid ];

That's the reason I kept the EDL records in a pre-allocated array to
begin with; to make the binary search possible. I recommend keeping it
that way.

This will wind up at the EDL record immediately preceeding the current
timer. Whether or not to actually perform this action is up to further
logic (if it's a seek, and we're in the middle of the seek region,
subtract the difference from the seek length, and jump ahead the smaller
amount... if it's a mute command, just execute it).

I also have some code that overhauls the muting system (there is a
muting state machine, which plays nice with human and EDL mute
requests).

I will hold off until after the feature freeze on this.

Mike

On Fri, 2003-01-17 at 05:25, Kövesdi György wrote:
> Hi,
> 
> Some enhancements for edl:
> - accepts hh:mm:ss time format
> - -edl option for mencoder too
> - allows overlaps
> - sorts the entries
> 
> K Gy
> ----
> 

> diff -NPaur MPlayer-20030117.original/Makefile MPlayer-20030117/Makefile
> --- MPlayer-20030117.original/Makefile	Sun Jan 12 00:08:11 2003
> +++ MPlayer-20030117/Makefile	Fri Jan 17 11:37:02 2003
> @@ -22,7 +22,7 @@
>  DO_MAKE = @ for i in $(SUBDIRS); do $(MAKE) -C $$i $@; done
>  endif
>  
> -SRCS_COMMON = cpudetect.c codec-cfg.c cfgparser.c my_profile.c spudec.c playtree.c playtreeparser.c asxparser.c vobsub.c subreader.c sub_cc.c find_sub.c m_config.c m_option.c parser-cfg.c m_struct.c
> +SRCS_COMMON = cpudetect.c codec-cfg.c cfgparser.c my_profile.c spudec.c playtree.c playtreeparser.c asxparser.c vobsub.c subreader.c sub_cc.c find_sub.c m_config.c m_option.c parser-cfg.c m_struct.c edl.c
>  SRCS_MENCODER = mencoder.c mp_msg-mencoder.c $(SRCS_COMMON) libao2/afmt.c divx4_vbr.c libvo/aclib.c libvo/osd.c libvo/sub.c libvo/font_load.c libvo/font_load_ft.c xvid_vbr.c parser-mecmd.c
>  SRCS_MPLAYER = mplayer.c mp_msg.c $(SRCS_COMMON) mixer.c parser-mpcmd.c
>  
> diff -NPaur MPlayer-20030117.original/cfg-mencoder.h MPlayer-20030117/cfg-mencoder.h
> --- MPlayer-20030117.original/cfg-mencoder.h	Sat Jan 11 21:32:46 2003
> +++ MPlayer-20030117/cfg-mencoder.h	Fri Jan 17 11:22:10 2003
> @@ -206,6 +206,7 @@
>  //	{"quiet", &quiet, CONF_TYPE_FLAG, 0, 0, 1, NULL},
>  	{"verbose", &verbose, CONF_TYPE_INT, CONF_RANGE|CONF_GLOBAL, 0, 100, NULL},
>  	{"v", cfg_inc_verbose, CONF_TYPE_FUNC, CONF_GLOBAL, 0, 0, NULL},
> +        {"edl", &edl_filename,  CONF_TYPE_STRING, 0, 0, 0, NULL},
>  //	{"-help", help_text, CONF_TYPE_PRINT, CONF_NOCFG, 0, 0, NULL},
>  //	{"help", help_text, CONF_TYPE_PRINT, CONF_NOCFG, 0, 0, NULL},
>  //	{"h", help_text, CONF_TYPE_PRINT, CONF_NOCFG, 0, 0, NULL},
> diff -NPaur MPlayer-20030117.original/edl.c MPlayer-20030117/edl.c
> --- MPlayer-20030117.original/edl.c	Thu Jan  1 01:00:00 1970
> +++ MPlayer-20030117/edl.c	Fri Jan 17 11:49:52 2003
> @@ -0,0 +1,104 @@
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#ifdef NEW_CONFIG
> +#include "m_option.h"
> +#include "m_config.h"
> +#include "parser-mecmd.h"
> +#else
> +#include "cfgparser.h"
> +#include "playtree.h"
> +#endif
> +
> +#include "edl.h"
> +
> +edl_record_ptr edl_list = NULL;
> +
> +static float convert_time(const char *time)
> +{
> + float sec;
> + int hour, min;
> + if (sscanf(time, "%d:%d:%f", &hour, &min, &sec) == 3) {
> +    return 3600*hour + 60*min + sec;
> + } else if (sscanf(time, "%d:%f", &min, &sec) == 2) {
> +    return 60*min + sec;
> + } else if (sscanf(time, "%f", &sec) == 1) {
> +    return sec;
> + }
> + printf("Error parsing edl file: <%s>\n", time);
> + return -1;
> +}
> +
> +
> +static int convert_action(const char *action)
> +{
> + if (!strcasecmp(action, "skip") || !strcmp(action, "0")) return EDL_SKIP;
> + if (!strcasecmp(action, "mute") || !strcmp(action, "1")) return EDL_MUTE;
> + printf("Invalid action string in the EDL file: <%s>\n", action);
> + return -1;
> +}
> +
> +
> +int open_edl_file(char *ifile) {
> + FILE *file;
> + char sor[100], from[100], to[100], action[100];
> +    edl_record_ptr actual, *prev;
> +
> +#ifdef DEBUG_EDL
> + printf("Opening EDL file... (%s)\n", ifile);
> +#endif
> + if ((file = fopen(ifile, "r")) == NULL) {
> +    printf("Error opening EDL file. EDL is not used. \n");
> +    return 0;
> + }
> + while (fgets(sor, sizeof(sor), file))
> +    if (sor[0] != '#' && sor[0] != '\n' && sor[0] != '\0') {
> +        float From, To;
> +        int Action;
> +
> +        sscanf(sor, "%s %s %s\n", from, to, action);
> +        From   = convert_time(from);
> +        To     = convert_time(to);
> +        Action = convert_action(action);
> +        if (From < 0 || To < 0 || Action < 0) break;
> +        if (From > To) {
> +            float temp = To;
> +            To = From;
> +            From = temp;
> +        }
> +        for (prev= &edl_list; *prev && (*prev)->start_sec < From; prev = &((*prev)->next)) ;
> +        actual = (edl_record_ptr)malloc(sizeof(struct edl_record));
> +        if (actual) {
> +            actual->start_sec = From;
> +            actual->stop_sec  = To;
> +            actual->action    = Action;
> +            actual->next      = *prev;
> +            *prev             = actual;
> +        } else {
> +            printf("Memory allocation error in EDL handler. \n");
> +            break;
> +        }
> + }
> +#ifdef DEBUG_EDL
> + for (actual=edl_list; actual; actual=actual->next) {
> +    printf("EDL: from %f to %f \n", actual->start_sec, actual->stop_sec);
> + }
> +#endif
> + fclose(file);
> + return 0;
> +}
> +
> +edl_record_ptr get_edl_entry(float actual_time)
> +{
> + edl_record_ptr fut;
> +
> + for (fut=edl_list; fut; fut=fut->next) {
> +    if (fut->start_sec <= actual_time && fut->stop_sec > actual_time) {
> +        return fut;
> +    }
> + }
> + return NULL;
> +}
> +
> +
> diff -NPaur MPlayer-20030117.original/mencoder.c MPlayer-20030117/mencoder.c
> --- MPlayer-20030117.original/mencoder.c	Fri Jan 17 00:03:06 2003
> +++ MPlayer-20030117/mencoder.c	Fri Jan 17 11:52:48 2003
> @@ -43,6 +43,10 @@
>  #include "playtree.h"
>  #endif
>  
> +#ifdef USE_EDL
> +#include "edl.h"
> +#endif
> +
>  #include "libmpdemux/stream.h"
>  #include "libmpdemux/demuxer.h"
>  #include "libmpdemux/stheader.h"
> @@ -96,6 +100,9 @@
>  int video_id=-1;
>  int dvdsub_id=-1;
>  int vobsub_id=-1;
> +#ifdef USE_EDL
> +static char* edl_filename=NULL;
> +#endif
>  static char* audio_lang=NULL;
>  static char* dvdsub_lang=NULL;
>  static char* spudec_ifo=NULL;
> @@ -403,6 +410,10 @@
>   if(!filelist) mencoder_exit(1, "error parsing cmdline");
>   m_entry_set_options(mconfig,&filelist[0]);
>   filename = filelist[0].name;
> +#ifdef USE_EDL
> + if (edl_filename) open_edl_file(edl_filename);
> +#endif
> + else printf("No EDL file given.\n");
>   // Warn the user if he put more than 1 filename ?
>  #else
>    playtree = play_tree_new();
> @@ -922,6 +933,26 @@
>        --play_n_frames;
>        if(play_n_frames<0) break;
>      }
> +
> +#ifdef USE_EDL
> +{
> +    edl_record_ptr edl;
> +    float actual=d_video->pts;
> +    while ((edl = get_edl_entry(actual)) != NULL) switch (edl->action) {
> +        case EDL_SKIP:
> +#ifdef DEBUG_EDL
> +            printf("\nEDL at %f: Seek to %f \n", actual, edl->stop_sec);
> +#endif
> +            actual=edl->stop_sec;
> +        break;
> +        case EDL_MUTE:
> +            // It is needed only to prevent endless loop:
> +            actual=edl->stop_sec;
> +        break;
> +    }
> +    if (actual != d_video->pts) demux_seek(demuxer, actual-d_video->pts, 0);
> +}
> +#endif
>  
>  if(sh_audio){
>      // get audio:
> diff -NPaur MPlayer-20030117.original/mplayer.c MPlayer-20030117/mplayer.c
> --- MPlayer-20030117.original/mplayer.c	Fri Jan 17 00:03:06 2003
> +++ MPlayer-20030117/mplayer.c	Fri Jan 17 11:52:48 2003
> @@ -308,10 +308,7 @@
>  #endif
>  
>  #ifdef USE_EDL
> -struct edl_record edl_records[ MAX_EDL_ENTRIES ];
> -int num_edl_records = 0;
>  FILE* edl_fd = NULL;
> -edl_record_ptr next_edl_record = NULL;
>  static char* edl_filename = NULL;
>  static char* edl_output_filename = NULL;
>  short edl_decision = 0;
> @@ -817,99 +814,19 @@
>      }
>  
>  #ifdef USE_EDL
> - {
> -   FILE* fd;
> -   char line[ 100 ];
> -   float start, stop, duration;
> -   int action;
> -   int next_edl_array_index = 0;
> -   int lineCount = 0;
> -   next_edl_record = edl_records;
> -   if( edl_filename ) {
> -     if( ( fd = fopen( edl_filename, "r" ) ) == NULL ) {
> -       printf( "Error opening EDL file [%s]!\n", edl_filename );
> -       next_edl_record->next = NULL;
> -     } else {
> -       while( fgets( line, 99, fd ) != NULL ) {
> -	 lineCount++;
> -	 if( ( sscanf( line, "%f %f %d", &start, &stop, &action ) ) == 0 ) {
> -	   printf( "Invalid EDL line: [%s]\n", line );
> -	 } else {
> -	   if( next_edl_array_index > 0 ) {
> -	     edl_records[ next_edl_array_index-1 ].next = &edl_records[ next_edl_array_index ];
> -	     if( start <= edl_records[ next_edl_array_index-1 ].stop_sec ) {
> -	       printf( "Invalid EDL line [%d]: [%s]\n", lineCount, line );
> -	       printf( "Last stop position was [%f]; next start is [%f]. Entries must be in chronological order and cannot overlap. Discarding EDL entry.\n", edl_records[ next_edl_array_index-1 ].stop_sec, start );
> -	       continue;
> -	     }
> -	   }
> -	   if( stop <= start ) {
> -	     printf( "Invalid EDL line [%d]: [%s]\n", lineCount, line );
> -	     printf( "Stop time must follow start time. Discarding EDL entry.\n" );
> -	     continue;
> -	   }
> -	   edl_records[ next_edl_array_index ].action = action;
> -	   if( action == EDL_MUTE ) {
> -	     edl_records[ next_edl_array_index ].length_sec = 0;
> -	     edl_records[ next_edl_array_index ].start_sec = start;
> -	     edl_records[ next_edl_array_index ].stop_sec = start;
> -	     next_edl_array_index++;
> -	     if( next_edl_array_index >= MAX_EDL_ENTRIES-1 ) {
> -	       break;
> -	     }
> -	     edl_records[ next_edl_array_index-1 ].next = &edl_records[ next_edl_array_index ];
> -	     edl_records[ next_edl_array_index ].action = EDL_MUTE;
> -	     edl_records[ next_edl_array_index ].length_sec = 0;
> -	     edl_records[ next_edl_array_index ].start_sec = stop;
> -	     edl_records[ next_edl_array_index ].stop_sec = stop;
> -	   } else {
> -	     edl_records[ next_edl_array_index ].length_sec = stop - start;
> -	     edl_records[ next_edl_array_index ].start_sec = start;
> -	     edl_records[ next_edl_array_index ].stop_sec = stop;
> -	   }
> -	   next_edl_array_index++;
> -	   if( next_edl_array_index >= MAX_EDL_ENTRIES-1 ) {
> -	     break;
> -	   }
> -	 }
> -       }
> -       if( next_edl_array_index > 0 ) {
> -	 edl_records[ next_edl_array_index-1 ].next = &edl_records[ next_edl_array_index ];
> -       }
> -       edl_records[ next_edl_array_index ].start_sec = -1;
> -       edl_records[ next_edl_array_index ].next = NULL;
> -       num_edl_records = ( next_edl_array_index );
> -     }
> -     fclose( fd );
> -   } else {
> -     next_edl_record->next = NULL;
> -   }
> -   if( edl_output_filename ) {
> -     if( edl_filename ) {
> -       printf( "Sorry; EDL mode and EDL output mode are mutually exclusive! Disabling all EDL functions.\n" );
> -       edl_output_filename = NULL;
> -       edl_filename = NULL;
> -       next_edl_record->next = NULL;
> -     } else {
> -       if( ( edl_fd = fopen( edl_output_filename, "w" ) ) == NULL ) {
> -	 printf( "Error opening file [%s] for writing!\n", edl_output_filename );
> -	 edl_output_filename = NULL;
> -	 next_edl_record->next = NULL;
> -       }
> -     }
> -   }
> -#ifdef DEBUG_EDL
> - {
> -   printf( "EDL Records:\n" );
> -   if( next_edl_record->next != NULL ) {
> -     while( next_edl_record->next != NULL ) {
> -       printf( "EDL: start [%f], stop [%f], action [%d]\n", next_edl_record->start_sec, next_edl_record->stop_sec, next_edl_record->action );
> -       next_edl_record = next_edl_record->next;
> -     }
> -     next_edl_record = edl_records;
> -   }
> - }
> -#endif
> + if (edl_filename) {
> +	if (edl_output_filename) {
> +		printf( "Sorry; EDL mode and EDL output mode are mutually exclusive! Disabling all EDL functions.\n" );
> +		edl_output_filename = NULL;
> +		edl_filename = NULL;
> +	} else {
> +		open_edl_file(edl_filename);
> +	}
> + } else if (edl_output_filename) {
> +	if( ( edl_fd = fopen( edl_output_filename, "w" ) ) == NULL ) {
> +		printf( "Error opening file [%s] for writing!\n", edl_output_filename );
> +		edl_output_filename = NULL;
> +	}
>   }
>  #endif
>  
> @@ -2200,27 +2117,25 @@
>  //================= EDL =========================================
>  
>  #ifdef USE_EDL
> - if( next_edl_record->next ) { // Are we (still?) doing EDL?
> -   if( sh_video->pts >= next_edl_record->start_sec ) {
> -     if( next_edl_record->action == EDL_SKIP ) {
> -       osd_function = OSD_FFW;
> -       abs_seek_pos = 0;
> -       rel_seek_secs = next_edl_record->length_sec;
> -#ifdef DEBUG_EDL
> -       printf( "\nEDL_SKIP: start [%f], stop [%f], length [%f]\n", next_edl_record->start_sec, next_edl_record->stop_sec, next_edl_record->length_sec );
> -#endif
> -       edl_decision = 1;
> -       next_edl_record = next_edl_record->next;
> -     } else if( next_edl_record->action == EDL_MUTE ) {
> -       mixer_mute();
> -#ifdef DEBUG_EDL
> -       printf( "\nEDL_MUTE: [%f]\n", next_edl_record->start_sec );
> -#endif
> -       edl_decision = 1;
> -       next_edl_record = next_edl_record->next;
> -     }
> -   }
> - }
> +{
> +    edl_record_ptr edl;
> +    float actual=d_video->pts;
> +    float original=actual;
> +    while ((edl = get_edl_entry(actual)) != NULL) switch (edl->action) {
> +        case EDL_SKIP:
> +	    osd_function = OSD_FFW;
> +	    abs_seek_pos = 0;
> +	    rel_seek_secs = (actual = edl->stop_sec) - original;
> +	    printf( "\nEDL_SKIP: start [%f], stop [%f], length [%f]\n", edl->start_sec, edl->stop_sec, edl->stop_sec-edl->start_sec );
> +	    edl_decision = 1;
> +        break;
> +        case EDL_MUTE:
> +	    mixer_mute();
> +	    printf( "\nEDL_MUTE: [%f]\n", edl->start_sec );
> +	    edl_decision = 1;
> +	break;
> +    }
> +}
>  #endif
>  
>  //================= Keyboard events, SEEKing ====================
> @@ -3017,20 +2932,7 @@
>        }
>    }
>  #ifdef USE_EDL
> -      {
> -	int x;
> -	if( !edl_decision ) {
> -	  for( x = 0; x < num_edl_records; x++ ) { // FIXME: do binary search
> -	    // Find first EDL entry where start follows current time
> -	    if( edl_records[ x ].start_sec >= sh_video->pts && edl_records[ x ].action != EDL_MUTE ) {
> -	      next_edl_record = &edl_records[ x ];
> -	      break;
> -	    }
> -	  }
> -	} else {
> -	  edl_decision = 0;
> -	}
> -      }
> +  edl_decision = 0;
>  #endif
>    rel_seek_secs=0;
>    abs_seek_pos=0;
> ----
> 

> _______________________________________________
> MPlayer-dev-eng mailing list
> MPlayer-dev-eng at mplayerhq.hu
> http://mplayerhq.hu/mailman/listinfo/mplayer-dev-eng
-- 
---------------------------------------- | ------------------------
Michael Halcrow                          | mhalcrow at byu.edu    
Research Assistant, Network Security Lab | Dept. of Comp. Science  
                                         | Brigham Young University
For a man to truly understand rejection, |
he must first be ignored by a cat.       |
---------------------------------------- | ------------------------
GnuPG Keyprint:  05B5 08A8 713A 64C1 D35D  2371 2D3C FDDA 3EB6 601D
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 232 bytes
Desc: This is a digitally signed message part
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20030117/1e26b5d8/attachment.pgp>


More information about the MPlayer-dev-eng mailing list