[MPlayer-dev-eng] DVDNAV Next Issues
Nico Sabbi
Nicola.Sabbi at poste.it
Sun Jan 27 10:36:44 CET 2008
Il Sunday 27 January 2008 00:37:45 Ötvös Attila ha scritto:
> Ezzel a dátummal: Saturday 26 January 2008 21.37.53 Benjamin Zores ezt írta:
> Hi Benjamin Zores!
>
> > >> Can you try to rework the audio change and sub change in 2 different
> > >> atomic patches that will avoid any code duplication ?
> > >
>
> I make dvdnav aid patch.
>
> Best regard.
> Attila
>
> Index: stream/stream_dvdnav.c
> ===================================================================
> --- stream/stream_dvdnav.c (revision 25852)
> +++ stream/stream_dvdnav.c (working copy)
> @@ -29,9 +29,16 @@
> NAV_FLAG_WAIT_READ_AUTO = 1 << 4, /* wait read auto mode */
> NAV_FLAG_WAIT_READ = 1 << 5, /* suspend read from stream */
> NAV_FLAG_VTS_DOMAIN = 1 << 6, /* vts domain */
> + NAV_FLAG_AUDIO_CHANGED = 1 << 7, /* audio stream change event */
> } dvdnav_state_t;
>
> typedef struct {
> + int title;
> + int mp_aid;
> + int nav_aid;
> +} title_lang_t;
> +
> +typedef struct {
> dvdnav_t * dvdnav; /* handle to libdvdnav stuff */
> char * filename; /* path */
> unsigned int duration; /* in milliseconds */
> @@ -41,6 +48,8 @@
> dvdnav_highlight_event_t hlev;
> int still_length; /* still frame duration */
> unsigned int state;
> + int title_lang_num;
> + title_lang_t* title_lang;
> } dvdnav_priv_t;
>
> extern char *dvd_device;
> @@ -322,6 +331,7 @@
> dvdnav_vts_change_event_t *vts_event = (dvdnav_vts_change_event_t *)s->buffer;
> mp_msg(MSGT_CPLAYER,MSGL_INFO, "DVDNAV, switched to title: %d\r\n", vts_event->new_vtsN);
> priv->state |= NAV_FLAG_CELL_CHANGED;
> + priv->state |= NAV_FLAG_AUDIO_CHANGED;
> priv->state &= ~NAV_FLAG_WAIT_SKIP;
> priv->state &= ~NAV_FLAG_WAIT;
> s->end_pos = 0;
> @@ -338,6 +348,7 @@
> }
> case DVDNAV_CELL_CHANGE: {
> priv->state |= NAV_FLAG_CELL_CHANGED;
> + priv->state |= NAV_FLAG_AUDIO_CHANGED;
> priv->state &= ~NAV_FLAG_WAIT_SKIP;
> priv->state &= ~NAV_FLAG_WAIT;
> if (priv->state & NAV_FLAG_WAIT_READ_AUTO)
> @@ -349,6 +360,10 @@
> }
> }
> break;
> + case DVDNAV_AUDIO_STREAM_CHANGE: {
> + priv->state |= NAV_FLAG_AUDIO_CHANGED;
> + }
> + break;
> }
> }
> mp_msg(MSGT_STREAM,MSGL_DBG2,"DVDNAV fill_buffer len: %d\n",len);
> @@ -899,7 +914,136 @@
> return 1;
> }
>
> +/**
> + * \brief Check if audio has changed
> + * \param stream: - stream pointer
> + * \param clear : - if true, then clear audio change flag
> + * \return 1 if audio has changed
> + */
> +int mp_dvdnav_is_audio_change(stream_t *stream, int clear) {
mp_dvdnav_audio_has_changed()
> + dvdnav_priv_t * priv=(dvdnav_priv_t*)stream->priv;
useless cast
> + if (!(priv->state & NAV_FLAG_AUDIO_CHANGED)) return 0;
> + if (clear)
bad style, use if(x), for(x), while(x) NEVER spaces before or
after parenthesis or brackets
> + priv->state &= ~NAV_FLAG_AUDIO_CHANGED;
> + return 1;
> +}
>
> +/**
> + * \brief dvdnav_aid_from_audio_num() returns the audio id corresponding to the logical number
> + * \param stream: - stream pointer
> + * \param audio_num: - logical number
> + * \return -1 on error, current subtitle id if successful
> + */
> +static int dvdnav_aid_from_audio_num(stream_t *stream, int audio_num) {
> + dvdnav_priv_t * priv=(dvdnav_priv_t*)stream->priv;
useless cast
> + int k;
> + uint8_t format, lg;
> +
> +#ifdef DVDNAV_FORMAT_AC3
DVDNAV_FORMAT_AC3 is always defined in out dvdnav
and other versions are not supported
> + //this macro is defined only in libdvdnav-cvs
> + for(k=0; k<32; k++) {
> + lg = dvdnav_get_audio_logical_stream(priv->dvdnav, k);
> + if (lg == 0xff) continue;
> + if (lg != audio_num) continue;
> + format = dvdnav_audio_stream_format(priv->dvdnav, lg);
> + switch(format) {
> + case DVDNAV_FORMAT_AC3:
> + return k+128;
> + case DVDNAV_FORMAT_DTS:
> + return k+136;
> + case DVDNAV_FORMAT_LPCM:
> + return k+160;
> + case DVDNAV_FORMAT_MPEGAUDIO:
> + return k;
> + default:
> + return -1;
> + }
> + }
> +#endif
> + return -1;
> +}
> +
duplicated code, refactor the existing function
> +/**
> + * \brief seek_title_lang() seek title language info
> + * \param priv: - priv pointer
> + * \param title: - title number
> + * \return title_lang_t pointer if found
> + */
bad name, express better what's supposed to do
> +static title_lang_t* seek_title_lang(dvdnav_priv_t * priv, int title) {
> + int i;
> +
> + if (!priv->title_lang)
> + return NULL;
> + if (priv->state & NAV_FLAG_VTS_DOMAIN)
> + return NULL;
> + for(i=0;i<priv->title_lang_num;i++)
> + if (priv->title_lang[i].title==title)
> + return &priv->title_lang[i];
> + return NULL;
> +}
> +
> +/**
> + * \brief new_title_lang() new title to language info
> + * \param priv: - priv pointer
> + * \param title: - title number
> + * \return title_lang_t new pointer
> + */
> +static title_lang_t* new_title_lang(dvdnav_priv_t * priv, int title) {
> + title_lang_t* title_lang;
> +
> + priv->title_lang_num++;
> + if (priv->title_lang)
> + priv->title_lang=realloc(priv->title_lang,
> + priv->title_lang_num*sizeof(title_lang_t));
> + else
> + priv->title_lang=malloc(priv->title_lang_num*sizeof(title_lang_t));
bad bad bad. Unchecked alloc()s, unconditional increments...
always use realloc()
> + title_lang=&priv->title_lang[priv->title_lang_num-1];
> + title_lang->title=title;
> + title_lang->mp_aid=-1;
> + title_lang->nav_aid=-1;
> + return title_lang;
> +}
> +
> +/**
> + * \brief set "mplayer" audio to title lang
> + * \param stream: - stream pointer
> + * \param audio_id: - audio number
> + */
> +void mp_dvdnav_set_aid(stream_t *stream, int audio_id) {
> + dvdnav_priv_t* priv=(dvdnav_priv_t*)stream->priv;
> +
> + title_lang_t* title_lang = seek_title_lang(priv,priv->title);
> + if (priv->state & NAV_FLAG_VTS_DOMAIN)
> + return;
> + if (!title_lang)
> + title_lang=new_title_lang(priv,priv->title);
check the result of your actions, please.
In case you didn't notice I'm for the paranoid kind of coding
> + title_lang->mp_aid=audio_id;
> +}
> +
> +/**
> + * \brief get current audio stream id
> + * \param stream: - stream pointer
> + * \return audio number
> + */
> +int mp_dvdnav_get_current_audio(stream_t *stream) {
> + dvdnav_priv_t* priv=(dvdnav_priv_t*)stream->priv;
> + int audio_id;
> + title_lang_t* title_lang = seek_title_lang(priv,priv->title);
> +
> + audio_id = dvdnav_get_active_audio_stream(priv->dvdnav);
> + audio_id = dvdnav_aid_from_audio_num(stream, audio_id);
> + if (priv->state & NAV_FLAG_VTS_DOMAIN)
> + return audio_id;
> + if (!title_lang)
> + title_lang=new_title_lang(priv,priv->title);
> + if (title_lang->nav_aid==audio_id && title_lang->mp_aid!=-1) {
> + title_lang->nav_aid=audio_id;
> + return title_lang->mp_aid;
> + }
> + title_lang->nav_aid=audio_id;
> + return audio_id;
> +}
confuses me...
> +
> const stream_info_t stream_info_dvdnav = {
> "DVDNAV stream",
> "null",
> Index: stream/stream_dvdnav.h
> ===================================================================
> --- stream/stream_dvdnav.h (revision 25852)
> +++ stream/stream_dvdnav.h (working copy)
> @@ -27,5 +27,8 @@
> int mp_dvdnav_skip_wait (stream_t *stream);
> void mp_dvdnav_read_wait (stream_t *stream, int mode, int automode);
> int mp_dvdnav_cell_has_changed (stream_t *stream, int clear);
> +int mp_dvdnav_is_audio_change(stream_t *stream, int clear);
> +void mp_dvdnav_set_aid(stream_t *stream, int audio_id);
> +int mp_dvdnav_get_current_audio(stream_t *stream);
>
> #endif /* MPLAYER_STREAM_DVDNAV_H */
> Index: mplayer.c
> ===================================================================
> --- mplayer.c (revision 25852)
> +++ mplayer.c (working copy)
> @@ -1909,6 +1909,47 @@
> if (decoded_frame && mpctx->nav_smpi != decoded_frame)
> mpctx->nav_smpi = mp_dvdnav_copy_mpi(mpctx->nav_smpi,decoded_frame);
> }
> +
> +/// Switch audio stream of DVDNAV
> +static int mp_dvdnav_switch_audio(void) {
> + int new_aid=mp_dvdnav_get_current_audio(mpctx->stream);
> + int old_aid=mpctx->demuxer->audio->id;
> + int aid;
> +
> + if (new_aid<0) {
> + demuxer_switch_audio(mpctx->demuxer, new_aid);
> + return 1;
> + }
> + if (new_aid==old_aid)
> + return 1;
> + aid = demuxer_switch_audio(mpctx->demuxer, new_aid);
> + if (aid!=new_aid)
> + return 0;
> + if (mpctx->demuxer->audio->id==-2) {
> + mpctx->demuxer->audio->id=-1;
> + return 1;
> + }
> + if (old_aid & 0x0F == new_aid & 0x0F) // codec unchanged
> + return 1;
> + if (new_aid == -2 || (new_aid > -1 && old_aid != -2 &&
> + mpctx->demuxer->audio->id != old_aid)) {
> + uninit_player(INITED_AO | INITED_ACODEC);
> + if (audio_id > -1 && mpctx->demuxer->audio->id != old_aid) {
> + sh_audio_t *sh2;
> + sh2 = mpctx->demuxer->a_streams[mpctx->demuxer->audio->id];
> + if (sh2) {
> + sh2->ds = mpctx->demuxer->audio;
> + mpctx->sh_audio = sh2;
> + reinit_audio_chain();
> + }
> + }
> + }
> + if (mpctx->demuxer->audio->id==-2)
> + mpctx->demuxer->audio->id=-1;
> + mp_dvdnav_is_audio_change(mpctx->stream,1);
> + return 1;
> +}
> +
> #endif /* USE_DVDNAV */
this code shouldn't be anymore needed after Ben's last commit
>
> static void adjust_sync_and_print_status(int between_frames, float timing_error)
> @@ -3765,6 +3806,9 @@
>
> #ifdef USE_DVDNAV
> if (mpctx->stream->type == STREAMTYPE_DVDNAV) {
> + if (mp_dvdnav_is_audio_change(mpctx->stream,0))
> + if (mp_dvdnav_switch_audio())
> + mp_dvdnav_is_audio_change(mpctx->stream,1);
> nav_highlight_t hl;
> mp_dvdnav_get_highlight (mpctx->stream, &hl);
> osd_set_nav_box (hl.sx, hl.sy, hl.ex, hl.ey);
> Index: command.c
> ===================================================================
> --- command.c (revision 25852)
> +++ command.c (working copy)
> @@ -853,6 +853,10 @@
> reinit_audio_chain();
> }
> }
> +#ifdef USE_DVDNAV
> + if (mpctx->stream->type == STREAMTYPE_DVDNAV)
> + mp_dvdnav_set_aid(mpctx->stream, audio_id);
> +#endif
> mp_msg(MSGT_IDENTIFY, MSGL_INFO, "ID_AUDIO_TRACK=%d\n", audio_id);
> return M_PROPERTY_OK;
> default:
More information about the MPlayer-dev-eng
mailing list