[MPlayer-dev-eng] bug in demux_mkv

[Reimar Döffinger]

Hello,
On Sun, Jul 15, 2007 at 09:37:08AM -0700, Sean D'Epagnier wrote:
> On Sun, Jul 15, 2007 at 10:02:52AM +0200, Reimar D?ffinger wrote:
> > Index: libmpdemux/demux_mkv.c
> > ===================================================================
> > --- libmpdemux/demux_mkv.c	(revision 23784)
> > +++ libmpdemux/demux_mkv.c	(working copy)
> > @@ -2520,7 +2520,7 @@
> >    else if (demuxer->audio->id != -2)  /* -2 = no audio at all */
> >      track = demux_mkv_find_track_by_num (mkv_d, demuxer->audio->id,
> >                                           MATROSKA_TRACK_AUDIO);
> > -  else
> > +  if (!track)
> >      {
> >        mp_msg (MSGT_DEMUX, MSGL_INFO, MSGTR_MPDEMUX_MKV_NoAudioTrackFound);
> >        demuxer->audio->id = -2;
> 
> This test is really not required since we know track is always NULL, but it
> may make the code clearer.

Seems like you missed the removal if the "else" ?? And yes, I did test
this patch and it does fix the only problem I have with latest SVN.

> > @@ -2534,7 +2534,7 @@
> >            continue;
> >        if(demuxer->a_streams[track->tnum])
> >          {
> > -          if(track && mkv_d->tracks[i] == track)
> > +          if(mkv_d->tracks[i] == track)
> >              {
> >                demuxer->audio->id = track->tnum;
> >                demuxer->audio->sh = demuxer->a_streams[track->tnum];
> 
> It's true it makes no sense to test if track is NULL here since it already
> crashed at "if(demuxer->a_streams[track->tnum])" if it was NULL.
> 
> There is still nothing in this patch I can see that tests if track is NULL
> before hitting "if(demuxer->a_streams[track->tnum])"

All this code is only executed if demuxer->audio->id != -2.

> It was mentioned that mplayer does correctly autodetect the audio track.
> I have both 64 and 32bit x86 systems.  Anyone else able to run:
> 
> mplayer http://people.freedesktop.org/~geckosenator/test.mkv
> 
> Tell us if it crashes or not.  It does for me.

It does not crash. Without the patch
mplayer -aid 1 http://people.freedesktop.org/~geckosenator/test.mkv
does crash though.

Greetings,
Reimar Döffinger