[FFmpeg-devel] [PATCH] Revert "Merge commit '741b494fa8cd28a7d096349bac183893c236e3f9'"

[Michael Niedermayer]

On Thu, Dec 17, 2015 at 02:46:40PM +0100, Hendrik Leppkes wrote:
> On Thu, Dec 17, 2015 at 2:06 PM, Michael Niedermayer <michaelni at gmx.at> wrote:
> > From: Michael Niedermayer <michael at 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


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

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- 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/20151219/79e40e37/attachment.sig>