[FFmpeg-devel] [PATCH 3/4] fate: add -fflags +bitexact to the relevant targets

[Michael Niedermayer]

On Sat, Aug 22, 2015 at 09:21:09PM -0400, Ronald S. Bultje wrote:
> Hi,
> 
> On Sat, Aug 22, 2015 at 8:21 PM, Michael Niedermayer <michael at niedermayer.cc
> > wrote:
> 
> > On Sat, Aug 22, 2015 at 07:12:13PM -0400, Ronald S. Bultje wrote:
> > > Hi,
> > >
> > > On Sat, Aug 22, 2015 at 6:54 PM, Michael Niedermayer
> > <michael at niedermayer.cc
> > > > wrote:
> > >
> > > > On Sat, Aug 22, 2015 at 07:57:33PM +0200, Andreas Cadhalpun wrote:
> > > > > This fixes fate with FF_API_LAVF_BITEXACT disabled.
> > > > >
> > > > > Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> > > > > ---
> > > > >  tests/Makefile           |  2 +-
> > > > >  tests/fate-run.sh        | 18 +++++++++---------
> > > > >  tests/fate/mpeg4.mak     |  2 +-
> > > > >  tests/fate/subtitles.mak |  4 ++--
> > > > >  tests/fate/vqf.mak       |  2 +-
> > > > >  5 files changed, 14 insertions(+), 14 deletions(-)
> > > > >
> > > > > diff --git a/tests/Makefile b/tests/Makefile
> > > > > index 06ed011..4791ade 100644
> > > > > --- a/tests/Makefile
> > > > > +++ b/tests/Makefile
> > > > > @@ -48,7 +48,7 @@ tests/data/ffprobe-test.nut: ffmpeg$(EXESUF)
> > > > tests/test_copy.ffmeta
> > > > >       $(M)$(TARGET_EXEC) $(TARGET_PATH)/$< \
> > > > >          -f lavfi -i "aevalsrc=sin(400*PI*2*t):d=0.125[out0];
> > > > testsrc=d=0.125[out1]; testsrc=s=100x100:d=0.125[out2]" \
> > > > >          -f ffmetadata -i $(TARGET_PATH)/tests/test_copy.ffmeta \
> > > > > -        -flags +bitexact -map 0:0 -map 0:1 -map 0:2 -map_metadata 1
> > \
> > > > > +        -flags +bitexact -fflags +bitexact -map 0:0 -map 0:1 -map
> > 0:2
> > > > -map_metadata 1 \
> > > > >          -map_metadata:s:0 1:s:0 -map_metadata:s:1 1:s:1 \
> > > > >          -vcodec rawvideo -acodec pcm_s16le \
> > > > >          -y $(TARGET_PATH)/$@ 2>/dev/null
> > > > > diff --git a/tests/fate-run.sh b/tests/fate-run.sh
> > > > > index 7aa5df2..c55609d 100755
> > > > > --- a/tests/fate-run.sh
> > > > > +++ b/tests/fate-run.sh
> > > > > @@ -102,11 +102,11 @@ ffmpeg(){
> > > > >  }
> > > > >
> > > > >  framecrc(){
> > > > > -    ffmpeg "$@" -flags +bitexact -f framecrc -
> > > > > +    ffmpeg "$@" -flags +bitexact -fflags +bitexact -f framecrc -
> > > > >  }
> > > >
> > > > is it wanted to basically require specifying the bitexact flag twice
> > > > to get bitexact output ?
> > > >
> > > > is this an improvment of the command line options ?
> > >
> > >
> > > It is an improvement to the AVFormatContext/AVCodecContext options.
> >
> > >
> > > If you want to improve commandline options, maybe introduce a -bitexact
> > > option for ffmpeg_opt.c? You could make it apply to sws/swr also at the
> > > same time. But that does not belong in AVOptions.
> >
> > that would be ideal, yes
> >
> > The problem is that if you use "-flags +bitexact" currently there
> > is no hint that this will become insufficient for generating bitexact
> > files. That is, using this option for generating bitexact containers
> > is/was not deprecated in a way that is vissible to the user.
> > Yet with the bump this will stop working and not only that, its not
> > vissible to the user beyond that the files arent bitexact anymore.
> 
> 
> Then add a warning.
> 
> Also, next time, don't figure this sort of stuff out a few days before a
> version bump. You merged this patch. Think about it before you merge.

Merging a patch that adds some #if xyz, is not the same as planing
to remove the code covered by that #if in the next version bump.

In general i intended to drop things once
* It builds and works correctly (or is trivial to change so it does)
* has no users
-OR-
is very significant work to maintain and conflicts with future changes

Making the FF_API* == 0 actually work always was the communities job
and if noone (myself included) did than it was postponed IIRC

Now many people want to drop instead of further postponing many
things. These people will ultimately have to do the work that is
required to make that happen.

Iam just trying to help by pointing out potential problems that i
realize are there. I might also help fixing one or the other thing
related to the FF_API* stuff if it feels interresting or fun to do.
But this kind of cleanup&removial is not really my area of interrest

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

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150823/c2e97386/attachment.sig>