[MPlayer-dev-eng] [PATCH] dvdnav part 2 - still frames

[Diego Biurrun]

On Sun, Aug 05, 2007 at 02:00:48PM +0200, Ötvös Attila wrote:
> 2007. augusztus 5. 12.52 dátummal Carl Eugen Hoyos ezt írta:
> >
> > I wonder if Nico maybe meant *all* #if0's when he wrote "those #if0 are
> > useless, better remove the enclosed code"?
> >
> > Carl Eugen
> I correction and remove uninit and depend code.
> 
> --- stream/stream_dvdnav.c	(revision 24014)
> +++ stream/stream_dvdnav.c	(working copy)
> @@ -239,6 +272,11 @@
>      int event;
>  
>      dvdnav_priv_t* priv=s->priv;
> +   // is suspend read
> +   if (priv->status & NAV_FLAG_WAIT_READ) {
> +      len=-1;
> +      return len;
> +    }

wrong indentation, please don't indent by 3 spaces

> @@ -431,26 +485,24 @@
>  
>        dvdnav_current_title_info(nav, &title, &part);
>        if(title) {
> -        if(dvdnav_menu_call(nav, DVD_MENU_Part) == DVDNAV_STATUS_OK
> -           || dvdnav_menu_call(nav, DVD_MENU_Title) == DVDNAV_STATUS_OK) {
> -          reset = 1;
> -          break;
> -        }
> +        if((status=dvdnav_menu_call(nav, DVD_MENU_Part)) == 
> +	    DVDNAV_STATUS_OK)
> +	  break;
> +        if((status=dvdnav_menu_call(nav, DVD_MENU_Title)) == 
> +	    DVDNAV_STATUS_OK)
> +	  break;

Wrong indentation, also, I think if condition could be kept on one line.

> @@ -647,6 +699,69 @@
>  
> +/**
> + * \brief mp_dvdnav_isreallyeof() query is really dvd stream eof

This is ungrammatical.  What is the subject of this sentence?  Query?
What are you trying to say?

> + * \param stream: - stream pointer
> + * \return 1 if really eof

If what is EOF?

> +/**
> + * \brief mp_dvdnav_still_skip() still skip

Skip still frame(s)?

> + * \param stream: - stream pointer
> + * \return 1 if success

on success

Shouldn't successful function calls return 0?

> +/**
> + * \brief mp_dvdnav_wait_skip() wait skip
> + * \param stream: - stream pointer
> + * \return 1 if success
> + */

ditto

> +/**
> + * \brief mp_dvdnav_iscellchage() query cell change flag, clear this flag

chaNge

> +/**
> + * \brief mp_dvdnav_wait_read() set wait mode
> + * \param stream  : - stream pointer
> + * \param mode    : - if true then suspensed block read

suspend

Suspense is the feeling you get while watching a thriller.

> --- stream/stream_dvdnav.h	(revision 24014)
> +++ stream/stream_dvdnav.h	(working copy)
> @@ -14,18 +14,27 @@
>  
> +/* flags to status */

status flags

> +#define NAV_FLAG_STREAM_CHANGE  0x0040  /* stream change flag: title, part, audio or spu */

SPU

> +#define NAV_FLAG_VTS_DOMAIN     0x0080  /* is vts domain */

vts domain

> --- mplayer.c	(revision 24014)
> +++ mplayer.c	(working copy)
> @@ -1682,6 +1696,180 @@
>  
> +    if (!frommpi) return NULL;
> +    // B-frame don't store

Do not store B-frames.

> +// video frame pre process: before decode_video()

preprocess

> +static void* mp_dvdnav_decode_video_pre(int *in_size,
> +    unsigned char **start, mp_image_t* decoded_frame) {
> +    // is change in dvdnav stream

change in dvdnav stream

> +// if wait or still frame (in_size==-1) and empty last
> +// video frame and valid last decoded image and
> +// no repeat decode frame then decoded frame
> +// image = stored last decoded frame image

This sentence is ungrammatical, where is the verb?

> +    if (*in_size<0 && !mpctx->last_buffer && mpctx->smpi &&
> +	    !mpctx->libmpeg2_count)
> +        decoded_frame=mpctx->smpi;
> +    mpctx->dup_frame=0;
> +// id wait or still frame and valid last read video frame

I don't understand.

> +    if (*in_size<0 && mpctx->last_buffer) {
> +        // set duplicate mode
> +        mpctx->dup_frame=1;
> +        // is first call repeat number = 5

Leave out the is.

> +        // is valid last video buffer (required libmpeg2!)

Leave out the is.

> +        if (mpctx->last_start) {
> +             // set start, in_size and copy frame data
> +             // from stored frame

Capitalize, add a period at the end.

> +// video frame post process: after decode_video()

postprocess

> +    // is wait or still frame and no redecode last frame

Leave out the is.

> +        // is part end

Leave out the is.

> +        // is duplicate mode

ditto

> +            // is last reprocess frame

ditto

> +                // unsuccessful decode video frame and is max possible

I don't understand

> +                    mp_msg(MSGT_CPLAYER,MSGL_ERR,
> +                        "Can't decode still frame.\n"
> +                        "Please try to -vc ffmpeg12 options.\n");

This is ungrammatical, where is the verb that needs to follow "to".
What are you trying to say?

> +            // allocate buffer and store readed video frame data

read

> +        // if decoded ok and no stored image then store image

Frame decoded OK?

> @@ -2010,6 +2198,17 @@
> +    if (mpctx->stream->type==STREAMTYPE_DVDNAV && in_size < 0) {
> +        // is really eof?

Leave out the is.

> @@ -2040,8 +2239,22 @@
> +    if (mpctx->stream->type==STREAMTYPE_DVDNAV)
> +        // call dvdnav pre video process

video preprocess?

> +    // if is stored image then no decode_video

ungrammatical, no subject

> +    if (mpctx->stream->type==STREAMTYPE_DVDNAV)
> +        // call dvdnav post video process

video postprocess?

Diego

P.S.: Your mails would be much more readable if you left an empty line
between your answer and the quoted text.