[MPlayer-dev-eng] [PATCH] Allow EDLs to work on audio files (fwd)

[Alexander Strasser]

Hi,

  just some notes from quickly looking at the diff without any
testing. Excuse me, if the patch was already applied and some
of my comments are obsolete. I don't get to follow the project
as closely as I would like to.

Stéphane Doyon wrote:
> Saw no response to this, resending.
[...]
> I had need to use an EDL on an audio file and was surprised to get this
> error:
> "Cannot use EDL without video, disabling."
> 
> It wasn't obvious to me why there should be such a constraint. I
> took an admittedly quick look at the code, and found only the
> position was obtained from the video stream, and there seems no
> reason it can't use the audio position just as well. I made the
> simple change to use the audio position when there's no video, and
> it appears to work.
[...]

> --- mplayer.c.orig	2011-05-28 18:16:16.862490052 -0400
> +++ mplayer.c	2011-05-28 18:32:53.274490061 -0400
> @@ -2663,7 +2663,13 @@
>          return;
>      }
>  
> -    if (!mpctx->sh_video) {
> +    // Get position from video if available else from audio.
> +    double pts;

  This declaration should be moved at the beginning of a scope,
if I don't misread the surrounding diff.

> +    if (mpctx->sh_video) {
> +        pts = mpctx->sh_video->pts;
> +    } else if (mpctx->sh_audio) {
> +        pts = mpctx->sh_audio->pts;
> +    } else {
>          mp_msg(MSGT_CPLAYER, MSGL_ERR, MSGTR_EdlNOsh_video);

  This message should probably be changed to read: "Cannot use
EDL without video nor audio stream available". This makes me think
if the code path is or should at all be reachable after the patch.
But still coding it defensively should be preferred IMHO.

  The trailing patch is not snipped, to allow potential referencing
in reply to this mail.

>          free_edl(edl_records);
>          next_edl_record = NULL;
> @@ -2681,19 +2687,18 @@
>          // Find next record, also skip immediately if we are already
>          // inside any record
>          while (next_edl_record) {
> -            if (next_edl_record->start_sec > mpctx->sh_video->pts)
> +            if (next_edl_record->start_sec > pts)
>                  break;
> -            if (next_edl_record->stop_sec >= mpctx->sh_video->pts) {
> +            if (next_edl_record->stop_sec >= pts) {
>                  if (edl_backward) {
>                      mpctx->osd_function = OSD_REW;
>                      edl_decision  = 1;
>                      abs_seek_pos  = 0;
> -                    rel_seek_secs = -(mpctx->sh_video->pts -
> -                                      next_edl_record->start_sec +
> +                    rel_seek_secs = -(pts - next_edl_record->start_sec +
>                                        edl_backward_delay);
>                      mp_msg(MSGT_CPLAYER, MSGL_DBG4, "EDL_SKIP: pts [%f], "
>                                                      "offset [%f], start [%f], stop [%f], length [%f]\n",
> -                           mpctx->sh_video->pts, rel_seek_secs,
> +                           pts, rel_seek_secs,
>                             next_edl_record->start_sec, next_edl_record->stop_sec,
>                             next_edl_record->length_sec);
>                      return;
> @@ -2711,15 +2716,15 @@
>      }
>  
>      if (next_edl_record &&
> -        mpctx->sh_video->pts >= next_edl_record->start_sec) {
> +        pts >= next_edl_record->start_sec) {
>          if (next_edl_record->action == EDL_SKIP) {
>              mpctx->osd_function = OSD_FFW;
>              edl_decision  = 1;
>              abs_seek_pos  = 0;
> -            rel_seek_secs = next_edl_record->stop_sec - mpctx->sh_video->pts;
> +            rel_seek_secs = next_edl_record->stop_sec - pts;
>              mp_msg(MSGT_CPLAYER, MSGL_DBG4, "EDL_SKIP: pts [%f], offset [%f], "
>                                              "start [%f], stop [%f], length [%f]\n",
> -                   mpctx->sh_video->pts, rel_seek_secs,
> +                   pts, rel_seek_secs,
>                     next_edl_record->start_sec, next_edl_record->stop_sec,
>                     next_edl_record->length_sec);
>          } else if (next_edl_record->action == EDL_MUTE) {


  Alexander