[MPlayer-dev-eng] [PATCH] dvdnav patch to still frames

[Nico Sabbi]

Ötvös Attila wrote:
> Hi All!
> 
> This patch realize dvdnav still frames.
> 
> Best regards.
> Attila

I have some questions:

> 
> lang, *dvdsub_lang;
>  
>  static struct stream_priv_s {
>    int track;
>    char* device;
>  } stream_priv_dflts = {
> -  0,
> +  -1,
>    NULL
>  };
>  

why?

>        case DVDNAV_CELL_CHANGE: {
> +        priv->status &= ~NAV_FLAG_WAIT_SKIP;
> +        priv->status |= NAV_FLAG_STREAM_CHANGE;
>          dvdnav_cell_change_event_t *ev =  (dvdnav_cell_change_event_t*)buf;
>          if(ev->pgc_length)
>            priv->duration = ev->pgc_length/90;
> +        if (dvdnav_is_domain_vts(priv->dvdnav))
> +          priv->status &= ~NAV_FLAG_VTS_DOMAIN;
> +        else
> +          priv->status |= NAV_FLAG_VTS_DOMAIN;

isn't this assignment inverted?
in a domain_vts shouldn't priv->status have NAV_FLAG_VTS_DOMAIN set?

> +
> +        nextstill = dvdnav_get_next_still_flag(priv->dvdnav);
> +        if (nextstill) {
> +          if (nextstill==0xff)
> +            priv->duration=0;
> +            else 
> +            priv->duration=nextstill * 1000;
> +          priv->still_length=nextstill;
> +        }

if(nextstill == 0xff)
    priv->still_length = priv->duration = 0;
else if(nextstill)
    priv->still_length = priv->duration=nextstill * 1000;

please, don't use if ( condition ) style: I dislike it

>  
>  
> -int mp_dvdnav_handle_input(stream_t *stream, int cmd, int *button) {
> +void mp_dvdnav_handle_input(stream_t *stream, int cmd, int *button) {

why don't you want me to reset the demuxer/decoders?

>  
> +/* status flags */
> +#define NAV_FLAG_EOFSTREAM      0x0001  /* stream eof flag */
> +#define NAV_FLAG_WAIT           0x0002  /* wait event */
> +#define NAV_FLAG_WAIT_SKIP      0x0004  /* wait skip disable */
> +#define NAV_FLAG_CELL_CHANGE    0x0008  /* cell change event */
> +#define NAV_FLAG_WAIT_READ_AUTO 0x0010  /* wait read auto mode (if opening demuxer then off else on) */
> +#define NAV_FLAG_WAIT_READ      0x0020  /* wait read flag (suspend read from dvdnav stream */
> +#define NAV_FLAG_STREAM_CHANGE  0x0040  /* stream change flag: title, part, audio or SPU */
> +#define NAV_FLAG_VTS_DOMAIN     0x0080  /* vts domain */
> +#define NAV_FLAG_SPU_SET        0x0100  /* spu_clut is valid */


you should explain in detail (in the .c file) the logic behind those
flags

>  
>  #endif
> Index: mplayer.c

I'd still like to know from Uoti or from someone if those changes to
mpi are acceptable (that I doubt) and if they clash with Uoti's
patches to handle stills