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

[Stefano Sabatini]

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?
 
> > Index: ffmpeg/ffplay.c
> > ===================================================================
> > --- ffmpeg.orig/ffplay.c	2009-02-28 00:45:00.000000000 +0100
> > +++ ffmpeg/ffplay.c	2009-02-28 00:46:11.000000000 +0100
> > @@ -1987,11 +1987,11 @@
> >          ic->streams[i]->discard = AVDISCARD_ALL;
> >          switch(enc->codec_type) {
> >          case CODEC_TYPE_AUDIO:
> > -            if ((audio_index < 0 || wanted_audio_stream-- > 0) && !audio_disable)
> > +            if (wanted_audio_stream-- >= 0 && !audio_disable)
> >                  audio_index = i;
> >              break;
> >          case CODEC_TYPE_VIDEO:
> > -            if ((video_index < 0 || wanted_video_stream-- > 0) && !video_disable)
> > +            if (wanted_video_stream-- >= 0 && !video_disable)
> >                  video_index = i;
> >              break;
> >          case CODEC_TYPE_SUBTITLE:
> 
> these require the default values to be changed to 0 or you break the code
> entirely

Default values are already set to 0, with the previous behaviour
wanted_video_stream=N and N <= 0 had the same behaviour as
wanted_video_stream = 1.

Regards.
-- 
FFmpeg = Faboulous & Fostering Mythic Ponderous Erotic Gadget