On Thu, Mar 5, 2015 at 10:32 AM, Michael Niedermayer <michae...@gmx.at> wrote:
> On Thu, Mar 05, 2015 at 10:23:02AM +0100, Robert Krüger wrote: > > On Wed, Mar 4, 2015 at 7:14 PM, Michael Niedermayer <michae...@gmx.at> > > wrote: > > > > > On Wed, Mar 04, 2015 at 07:07:35PM +0100, Robert Krüger wrote: > > > > On Wed, Mar 4, 2015 at 6:57 PM, Michael Niedermayer < > michae...@gmx.at> > > > > wrote: > > > > > > > > > On Wed, Mar 04, 2015 at 06:19:19PM +0100, Robert Krüger wrote: > > > > > > On Wed, Mar 4, 2015 at 5:34 PM, Michael Niedermayer < > > > michae...@gmx.at> > > > > > > wrote: > > > > > > > > > > > > > On Wed, Mar 04, 2015 at 03:40:09PM +0100, Robert Krüger wrote: > > > > > > > > On Wed, Mar 4, 2015 at 11:05 AM, Michael Niedermayer < > > > > > michae...@gmx.at> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > On Wed, Mar 04, 2015 at 10:22:26AM +0100, Robert Krüger > wrote: > > > > > > > > > > On Tue, Mar 3, 2015 at 9:27 PM, Michael Niedermayer < > > > > > > > michae...@gmx.at> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > On Tue, Mar 03, 2015 at 09:24:17PM +0100, Robert Krüger > > > wrote: > > > > > > > > > > > > On Tue, Mar 3, 2015 at 7:23 PM, Michael Niedermayer < > > > > > > > > > michae...@gmx.at> > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On Tue, Mar 03, 2015 at 06:17:22PM +0100, Robert > Krüger > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > This is based on an earlier patch by Derek > > > > > > > > > > > > > > > > > > > > > > > > > > please mention this in the commit message > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > OK, I will change that > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > that never went in because it > > > > > > > > > > > > > > was argumented earlier that api breakage is not > > > > > acceptable. > > > > > > > > > However, > > > > > > > > > > > that > > > > > > > > > > > > > > was more or less relaxed after Michael noted > that the > > > > > > > replaced > > > > > > > > > flag > > > > > > > > > > > had > > > > > > > > > > > > > > never been part of a release and since a number > of > > > people > > > > > > > seem to > > > > > > > > > > > agree, > > > > > > > > > > > > > > this is the better default, I am submitting this > > > patch > > > > > now, > > > > > > > to > > > > > > > > > have > > > > > > > > > > > it in > > > > > > > > > > > > > > before the upcoming release. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Let me know if that will be accepted and I will > > > modify > > > > > the > > > > > > > > > respective > > > > > > > > > > > > > fate > > > > > > > > > > > > > > tests as well. > > > > > > > > > > > > > > > > > > > > > > > > > > have you tested the generated mov and mp4 files > with > > > some > > > > > > > common > > > > > > > > > > > > > software packages ? > > > > > > > > > > > > > > > > > > > > > > > > > > checking random files on my disk it seems more than > > > half > > > > > the > > > > > > > mov > > > > > > > > > > > > > files contain a colr atom but i found just a > single mp4 > > > > > with a > > > > > > > colr > > > > > > > > > > > > > atom, so especially testing the compatibility of > the > > > mp4 > > > > > files > > > > > > > > > would > > > > > > > > > > > > > be optimal before this is changed > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > OK, I will do some tests with VLC, Quicktime, Final > Cut, > > > > > Final > > > > > > > Cut X, > > > > > > > > > > > > Premiere and After Effects and maybe something else I > > > find. > > > > > > > > > > > > > > > > > > > > > > thanks > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > I tried an mp4 file with a colr atom with VLC, Quicktime, > > > Final > > > > > Cut > > > > > > > X, > > > > > > > > > > Compressor, Premiere, After Effects and Adobe Media > Encoder. > > > > > None of > > > > > > > > > those > > > > > > > > > > had any problems with the file and colors looked normal > too. > > > > > > > > > > > > > > > > > > ok then please submit a patch that also updates fate > > > > > > > > > > > > > > > > > > > > > > > > > > here it is. > > > > > > > > > > > > > > > b/libavformat/movenc.c | 6 +-- > > > > > > > > b/libavformat/movenc.h | 2 - > > > > > > > > b/libavformat/version.h | 4 +- > > > > > > > > b/tests/fate/vcodec.mak | 8 > ++-- > > > > > > > > b/tests/ref/lavf/mov | 16 > > > ++++---- > > > > > > > > b/tests/ref/seek/lavf-mov | 44 > > > > > > > +++++++++++----------- > > > > > > > > b/tests/ref/vsynth/vsynth1-avui | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth1-dnxhd-1080i | 8 > ++-- > > > > > > > > b/tests/ref/vsynth/vsynth1-dnxhd-1080i-nocolr | 4 ++ > > > > > > > > b/tests/ref/vsynth/vsynth1-mpeg4 | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth1-prores | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth1-prores_ks | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth1-qtrle | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth1-qtrlegray | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth1-svq1 | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth2-avui | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth2-dnxhd-1080i | 8 > ++-- > > > > > > > > b/tests/ref/vsynth/vsynth2-dnxhd-1080i-nocolr | 4 ++ > > > > > > > > b/tests/ref/vsynth/vsynth2-mpeg4 | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth2-prores | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth2-prores_ks | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth2-qtrle | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth2-qtrlegray | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth2-svq1 | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth3-dnxhd-1080i-nocolr | 4 ++ > > > > > > > > b/tests/ref/vsynth/vsynth3-mpeg4 | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth3-prores | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth3-prores_ks | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth3-qtrle | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth3-svq1 | 4 +- > > > > > > > > b/tests/ref/vsynth/vsynth_lena-dnxhd-1080i-nocolr | 4 ++ > > > > > > > > tests/ref/vsynth/vsynth1-dnxhd-1080i-colr | 4 -- > > > > > > > > tests/ref/vsynth/vsynth2-dnxhd-1080i-colr | 4 -- > > > > > > > > tests/ref/vsynth/vsynth3-dnxhd-1080i-colr | 4 -- > > > > > > > > tests/ref/vsynth/vsynth_lena-dnxhd-1080i-colr | 4 -- > > > > > > > > 35 files changed, 101 insertions(+), 103 deletions(-) > > > > > > > > f6cafbe678f902fd7410b7dee69d9f6e04e591c1 > > > > > mov_write_colr_by_default.patch > > > > > > > > From f836ea0dcb181417b38a75302c1c5ffdeaa0fb4d Mon Sep 17 > 00:00:00 > > > > > 2001 > > > > > > > > From: =?UTF-8?q?Robert=20Kr=C3=BCger?= <krue...@lesspain.de> > > > > > > > > Date: Tue, 3 Mar 2015 18:11:54 +0100 > > > > > > > > Subject: [PATCH 1/2] mov: make writing colr the default > (based > > > on a > > > > > > > patch by > > > > > > > > derek buitenhuis) > > > > > > > > > > > > > > this breaks fate, the test needing the lena sample are missing > an > > > > > > > update > > > > > > > > > > > > > > > > > > > > Strange, I adjusted the lena sample tests and here make fate runs > > > without > > > > > > errors and I have no outgoing changes. Which test fails for you? > > > > > > > > > > i needed these: > > > > > > > > > > tests/ref/vsynth/vsynth_lena-avui | 4 ++-- > > > > > tests/ref/vsynth/vsynth_lena-dnxhd-1080i | 8 ++++---- > > > > > tests/ref/vsynth/vsynth_lena-mpeg4 | 4 ++-- > > > > > tests/ref/vsynth/vsynth_lena-prores | 4 ++-- > > > > > tests/ref/vsynth/vsynth_lena-prores_ks | 4 ++-- > > > > > tests/ref/vsynth/vsynth_lena-qtrle | 4 ++-- > > > > > tests/ref/vsynth/vsynth_lena-qtrlegray | 4 ++-- > > > > > tests/ref/vsynth/vsynth_lena-svq1 | 4 ++-- > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > also once one fixes this the fate scores differ between 32bit > and > > > > > > > 64bit on linux > > > > > > > --- tests/ref/vsynth/vsynth3-dnxhd-1080i-nocolr 2015-03-04 > > > > > > > 16:03:41.759112905 +0100 > > > > > > > +++ tests/data/fate/vsynth3-dnxhd-1080i-nocolr 2015-03-04 > > > > > > > 17:13:31.971201181 +0100 > > > > > > > @@ -1,4 +1,4 @@ > > > > > > > aaed55622ffd609d6b6114ee3da7f585 > > > > > > > *tests/data/fate/vsynth3-dnxhd-1080i-nocolr.mov > > > > > > > 3031911 tests/data/fate/vsynth3-dnxhd-1080i-nocolr.mov > > > > > > > -382fc519604abb5d87071bdce013cef9 > > > > > > > *tests/data/fate/vsynth3-dnxhd-1080i-nocolr.out.rawvideo > > > > > > > -stddev: 7.81 PSNR: 30.28 MAXDIFF: 61 bytes: 86700/ > > > 8670 > > > > > > > +cda7487f1c77f26df24668faa750257d > > > > > > > *tests/data/fate/vsynth3-dnxhd-1080i-nocolr.out.rawvideo > > > > > > > +stddev: 7.83 PSNR: 30.25 MAXDIFF: 61 bytes: 86700/ > > > 8670 > > > > > > > > > > > > > > iam starting to have a somewhat ungood feeling with doing this > > > change > > > > > > > short before the release > > > > > > > > > > > > > > > > > > > > I can understand that. What are the options? Make a release > without > > > the > > > > > > write_colr flag or make it with the flag and later deprecate it > in > > > favour > > > > > > of the no_colr flag? > > > > > > > > > > i guess we can just add the text "Experimental" or something > similar > > > > > to it and then later change it > > > > > removial would break a fate test which uses that flag > > > > > > > > > > > > > > OK, then I'll prepare a patch for after the release, although I > cannot > > > test > > > > the linux 32/64 bit thing. > > > > > > you dont have a 64bit box ? > > > i guess mingw32/64 would behave likewise > > > > > > > > No, I do not have a Linux box/environment. Only Mac. And also, I have not > > the slightest idea why something as high-level as changing/inverting the > > logic for inclusion of an atom should behave differently depending on > 32/64 > > bit. > > if you store the exact colorspace, this can end up using different > colorspaces when the resulting file is read later, that again can > lead to different codepathes being used and if now someone forgot some > bitexact related flag in a test it could cause diference between 32&64 > Thanks for the explanation! _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
