On Sat, Dec 19, 2015 at 10:35 AM, Michael Niedermayer <mich...@niedermayer.cc> wrote: > On Thu, Dec 17, 2015 at 02:46:40PM +0100, Hendrik Leppkes wrote: >> On Thu, Dec 17, 2015 at 2:06 PM, Michael Niedermayer <michae...@gmx.at> >> wrote: >> > From: Michael Niedermayer <mich...@niedermayer.cc> >> > >> > This fixes a regression of the sample from Ticket 2371 >> > >> > This reverts commit bc66451e5e903698ee0500faf04c1214f3dd157f, reversing >> > changes made to 9d1fb9ef313e0fb709ac4c35c7bf00264963fd85. >> > --- >> > libavcodec/h264.h | 8 +++++++ >> > libavcodec/h264_refs.c | 57 >> > ++++++++++++++++++++++++++--------------------- >> > libavcodec/h264_slice.c | 17 +++++++++++++- >> > 3 files changed, 56 insertions(+), 26 deletions(-) >> >> Reverting big commits to fix one sample and making future merges >> harder or impossible seems not like the best option, unless it can be >> shown that the entire change is flawed, and not just a small bug. >> Otherwise, it should be attempted to fix regressions the usual way - >> by fixing things, not reverting. >> >> Once reverted, the change will never come back, we all know that. >> >> At least thats IMHO. You are the maintainer of the H264 decoder, so if >> you want to revert it, feel free. > > Calculatig the default ref seems to take between 800 and 4700 or so > cpu cycles (messured with fate & START_TIMER) > > tests/data/fate/filter-paletteuse-sierra2_4a.err: 46958 decicycles in > h264_initialise_ref_list, 64 runs, 0 skips > tests/data/fate/h264-conformance-caba3_sony_c.err: 26963 decicycles in > h264_initialise_ref_list, 256 runs, 0 skips > tests/data/fate/h264-conformance-frext-hcafr4_hhi_a.err: 29306 decicycles in > h264_initialise_ref_list, 15 runs, 1 skips > tests/data/fate/h264-conformance-sva_cl1_e.err: 7956 decicycles in > h264_initialise_ref_list, 128 runs, 0 skips > ... > > with the default lists the time spend in Calculating them after the > first slice and including the checks needed to find out if they need > to be recalculated seem to be always below 500 cycles in fate > > tests/data/fate/h264-conformance-cvnlfi1_sony_c.err: 4196 decicycles in > CMP, 256 runs, 0 skips > tests/data/fate/h264-conformance-cvfi1_sony_d.err: 3976 decicycles in CMP, > 128 runs, 0 skips > tests/data/fate/h264-conformance-cvfi1_sony_d.err: 4142 decicycles in CMP, > 256 runs, 0 skips > tests/data/fate/h264-conformance-basqp1_sony_c.err: 381 decicycles in CMP, > 128 runs, 0 skips > tests/data/fate/h264-conformance-cvnlfi2_sony_h.err: 3757 decicycles in > CMP, 128 runs, 0 skips > tests/data/fate/h264-conformance-cvnlfi2_sony_h.err: 3687 decicycles in > CMP, 256 runs, 0 skips > tests/data/fate/h264-conformance-ci1_ft_b.err: 4588 decicycles in CMP, > 128 runs, 0 skips > tests/data/fate/h264-conformance-ci1_ft_b.err: 4754 decicycles in CMP, > 256 runs, 0 skips > tests/data/fate/h264-conformance-cvfc1_sony_c.err: 4146 decicycles in CMP, > 128 runs, 0 skips > tests/data/fate/h264-conformance-cabaci3_sony_b.err: 2739 decicycles in > CMP, 128 runs, 0 skips > tests/data/fate/h264-conformance-cabaci3_sony_b.err: 2732 decicycles in > CMP, 256 runs, 0 skips > tests/data/fate/h264-conformance-cabaci3_sony_b.err: 2878 decicycles in > CMP, 512 runs, 0 skips > tests/data/fate/h264-conformance-ba1_ft_c.err: 4266 decicycles in CMP, > 128 runs, 0 skips > tests/data/fate/h264-conformance-ba1_ft_c.err: 4358 decicycles in CMP, > 256 runs, 0 skips > tests/data/fate/h264-conformance-cvfi2_sony_h.err: 4347 decicycles in CMP, > 128 runs, 0 skips > tests/data/fate/h264-conformance-cvfi2_sony_h.err: 4179 decicycles in CMP, > 256 runs, 0 skips > > its possible the extra list causes harder to meassure slowdowns > elsewhere of course (cache hit/miss ratio, more fields in the context, > whatever) > > but for files with many small slices recalculating the same list > for each seems a waste of time to me. > > i also suspect that libav had a bug in their default ref list > code or at least their checks look less complete than ours prior > to the removial in both trees > > I think the decission about which way to go here should be yours > as you might (or might not) be affected by this causing extra work > in the future >
Libavs patch doesn't actually fix the sample, and considering there may be a slowdown, revert is fine with me. Its not like H264 is going to change substantially in the future. - Hendrik _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
