[MPlayer-dev-eng] [PATCH] EDL extensions, resubmit

[Reynaldo H. Verdejo Pinochet]

Hi Vlad

On Sat, Jun 19, 2010 at 06:03:18PM -0400, Vlad Seryakov wrote:
> Sorry, Reynaldo
> 
> Something with mailer or SPAM filter, i did not receive emails from the list, so i had to start a new one.
> 

Hope you got that fixed now.

> With indentation, this time i used spaces to be sure and tried not to introduce more cosmetics changes
> I tested with new If statements moved around and it works well

Thanks but you missed at least one important comment in my
previous email. The one asking you about the use of a + 2.

> [...]
> -static void edl_seek_reset(MPContext *mpctx)
> -{
> -    mpctx->edl_muted = 0;
> -    next_edl_record = edl_records;
> -
> -    while (next_edl_record) {
> -	if (next_edl_record->start_sec >= mpctx->sh_video->pts)
> -	    break;
> -
> -	if (next_edl_record->action == EDL_MUTE)
> -	    mpctx->edl_muted = !mpctx->edl_muted;
> -	next_edl_record = next_edl_record->next;
> -    }
> -    if ((mpctx->user_muted | mpctx->edl_muted) != mpctx->mixer.muted)
> -	mixer_mute(&mpctx->mixer);
> -}

This function removal would have been better handled as
a separate patch but you can leave it like it is this time
around. Please keep in mind for future contributions.

> [...]
> +                    edl_decision = 1;
> +                    abs_seek_pos = 0;
> +                    rel_seek_secs = -(mpctx->sh_video->pts - next_edl_record->start_sec + 2);

Line too long. Try not to go past 80.

Why two?

> [...]
> +                    mp_msg(MSGT_CPLAYER, MSGL_DBG4, "EDL_SKIP: pts [%f], offset [%f], "

Here too.

> +                           "start [%f], stop [%f], length [%f]\n", 
> +                           mpctx->sh_video->pts, rel_seek_secs, 
> +                           next_edl_record->start_sec, next_edl_record->stop_sec, 

Too. Thanks for the alignement attempt though.

> [...]
> +            next_edl_record = next_edl_record->next;
>  	}
> +	if ((mpctx->user_muted | mpctx->edl_muted) != mpctx->mixer.muted)
> +	    mixer_mute(&mpctx->mixer);
> +    }

Messed up indentation.

> +
> +    if (next_edl_record && mpctx->sh_video->pts >= next_edl_record->start_sec) {

Line too long.

> [...]
> -    edl_seek_reset(mpctx);
> +    // Delay EDL reset until we have valid PTS, this will be done in 
> +    // edl_update later

Needs rephrasing.


Its getting better. Thanks for your work!

--
Reynaldo