[FFmpeg-devel] [PATCH] Add documentation for -ast, -vst, -sst

[Michael Niedermayer]

On Sun, Mar 01, 2009 at 01:14:05AM +0100, Stefano Sabatini wrote:
> On date Saturday 2009-02-28 20:49:18 +0100, Michael Niedermayer encoded:
> > On Sat, Feb 28, 2009 at 12:57:02AM +0100, Stefano Sabatini wrote:
> > > On date Friday 2009-02-27 17:19:30 -0500, Ronald S. Bultje encoded:
> > > > Hi,
> > > > 
> > > > On Fri, Feb 27, 2009 at 4:18 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > > > > On Fri, Feb 27, 2009 at 03:26:19PM -0500, Ronald S. Bultje wrote:
> > > > >> On Thu, Feb 26, 2009 at 2:43 PM, Stefano Sabatini
> > > > >> <stefano.sabatini-lala at poste.it> wrote:
> > > > >> > On date Tuesday 2009-02-24 23:50:40 +0100, Stefano Sabatini encoded:
> > > > >> >> + at item -ast @var{audio_stream_number}
> > > > >> >> +Select the desired audio stream number, counting from 1.
> > > > >>
> > > > >> This has confused me all too often. Can we change this so it counts
> > > > >> like a regular number from 0?
> > > > >
> > > > > yes
> > > > 
> > > > that piece of code is full of crap btw, <0 for subtitles is seen as
> > > > "disable" but <0 for audio/video isn't. I added that also, which makes
> > > > the subtitle/video/audio selection code more alike. This would also
> > > > allow to remove -an/-vn at some point but that could also be done
> > > > after the release.
> > > > 
> > > > if you think this changes too much, I'll just apply the
> > > > "wanted_x_stream < 0" to "wanted_x_stream <= 0" *3 change.
> > > > 
> > > > Stefano, can you regen your documentation patch with this change taken
> > > > into account and apply that also?
> > > 
> > > Hi, thanks for noticing the issue, yes the code as it is is greatly
> > > obfuscated and subtlely broken.
> > > 
> > > I split your patch and done more simplifications.
> > > Summary:
> > > 
> > > * ffplay-fix-sst-logic.patch
> > >   When the user selects the subtitle stream N, if N is not 0 then it
> > >   is always selected N-1.
> > >   Patches fixes this.
> > > 
> > > * ffplay-simplify-sst-logic.patch
> > >   Simplify the logic, using the following reasoning:
> > > 
> > >   A & B & (C | A) =
> > >   A & B
> > > 
> > >   since if A & B then A is always true so (C | A) is always true.
> > >   Formal derivation below:
> > > 
> > >   A & B & (C | A) =
> > >   (A & B) & (C | A) =
> > >   (A & B & C) | (A & B & A) =
> > >   (A & B & C) | (A & B) =
> > >   A & B
> > > 
> > >   Thus the third condition (subtitle_index < 0) is unnecessary.
> > > 
> > > * ffplay-consistent-ast-vst.patch
> > >   Make the -ast and -vst behaviour consistent with that of -sst, that
> > >   is count starts from 0 and negative values are interpreted as
> > >   "disable".
> > > 
> > > * ffplay-document-xst.patch
> > >   Document the -ast, -vst, -sst options according to the new
> > >   (sane) behaviour.
> > > 
> > > Regards.
> > > -- 
> > > FFmpeg = Fundamentalist and Faboulous Murdering Portentous Erroneous Gem
> > 
> > > Index: ffmpeg/ffplay.c
> > > ===================================================================
> > > --- ffmpeg.orig/ffplay.c	2009-02-28 00:08:26.000000000 +0100
> > > +++ ffmpeg/ffplay.c	2009-02-28 00:31:10.000000000 +0100
> > > @@ -1996,7 +1996,7 @@
> > >              break;
> > >          case CODEC_TYPE_SUBTITLE:
> > >              if (wanted_subtitle_stream >= 0 && !video_disable &&
> > > -                    (subtitle_index < 0 || wanted_subtitle_stream-- > 0))
> > > +                    (subtitle_index < 0 || wanted_subtitle_stream-- >= 0))
> > >                  subtitle_index = i;
> > >              break;
> > >          default:
> > 
> > ok
> > 
> > 
> > > Index: ffmpeg/ffplay.c
> > > ===================================================================
> > > --- ffmpeg.orig/ffplay.c	2009-02-28 00:41:39.000000000 +0100
> > > +++ ffmpeg/ffplay.c	2009-02-28 00:45:00.000000000 +0100
> > > @@ -1995,8 +1995,7 @@
> > >                  video_index = i;
> > >              break;
> > >          case CODEC_TYPE_SUBTITLE:
> > > -            if (wanted_subtitle_stream >= 0 && !video_disable &&
> > > -                    (subtitle_index < 0 || wanted_subtitle_stream-- >= 0))
> > > +            if (wanted_subtitle_stream-- >= 0 && !video_disable)
> > >                  subtitle_index = i;
> > >              break;
> > >          default:
> > 
> > this patch is incorrect
> > but thanks for optimizing the bug
> 
> Uh? Please could you explain?

sorry i think ive misread, its ok

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090301/437f6e0b/attachment.pgp>