[MPlayer-dev-eng] [PATCH] vobsub fix for empty useless streams

[Ulion]

2007/9/19, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> Hello,
> On Sat, Sep 15, 2007 at 08:01:16PM +0800, Ulion wrote:
> > +/* get vobsub id by it's index in the valid streams */
>
> Comments should be doxygen compatible.

Fixed, please check it.

>
> > +extern int vobsub_get_id_by_index(void * /* vobhandle */, unsigned int /* index */);
>
> Stupidity in the surrounding code does not mean you should copy it.
> Please don't comment out the parameter names, just write them there
> normally.

Fixed.

>
> [...]
> > @@ -1165,8 +1167,11 @@
> >                   }
> >               }
> >               vob->spu_streams_current = vob->spu_streams_size;
> > -             while (vob->spu_streams_current-- > 0)
> > +             while (vob->spu_streams_current-- > 0) {
> >                   vob->spu_streams[vob->spu_streams_current].current_index = 0;
> > +                 if (vob->spu_streams[vob->spu_streams_current].id)
> > +                     ++vob->spu_valid_streams_size;
> > +             }
>
> This is potentially the big Achilles' heel of this patch: It assumes
> that we know all subtitle tracks on open. This it seems relies on a
> correct .idx file. Maybe this requirement is ok, though I'd be happy if
> someone could comment on it conclusively.
> But even then, it should be possible to select any stream with -vobsubid
> even when it is not in the .idx or .ifo or whatever file.

Fixed, if -vobsubid is setted and less than vob->spu_streams_size,
make it visible when loop subtitles.

>
> > @@ -1201,6 +1206,34 @@
> >      return (index < vob->spu_streams_size) ? vob->spu_streams[index].id : NULL;
> >  }
> >
> > +int vobsub_get_id_by_index(void *vobhandle, unsigned int index)
> > +{
> > +    vobsub_t *vob = (vobsub_t *) vobhandle;
>
> Useless cast (and I am aware that the surrounding code is full of
> those).

Since there's no compile warning without the case, removed.

>
> > +int vobsub_get_index_by_id(void *vobhandle, int id)
> > +{
> > +    vobsub_t *vob = (vobsub_t *) vobhandle;
> > +    int i, j;
> > +    if (vob == NULL || id < 0 || id >= vob->spu_streams_size
> > +            || vob->spu_streams[id].id == NULL)
>
> ?? That last condition I don't understand.

The last condition is removed now to make vobsub stream set by
-vobsubid can be looped.

>
> [...]
> >      if (source == SUB_SOURCE_VOBSUB) {
> > -     vobsub_id =
> > -         mpctx->global_sub_pos -
> > -         mpctx->global_sub_indices[SUB_SOURCE_VOBSUB];
> > +        vobsub_id = vobsub_get_id_by_index(vo_vobsub, mpctx->global_sub_pos -
> > +            mpctx->global_sub_indices[SUB_SOURCE_VOBSUB]);
>
> While I don't like them, the surrounding code uses tabs, so please leave
> them in.

Fixed.

Here's the fixed patch.

-- 
Ulion
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: vobsub_empty_sub_fix2.txt
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20070919/514a3b31/attachment.txt>