Hi all,
Rémi has requested a while ago that the TC resolve a disagreement around
his patches [1-8] changing the structure of the H.263 DCT dequant code.
The patches are intended to simplify adding arch-specific optimizations
and checkasm tests, however they add an indirect call and Andreas was
concerned about its effects on performance.
The TC investigated the problem and unanimously concluded the following:
* adding checkasm tests and simplifying future maintenance are
important goals that may be worth a small performance hit;
* nevertheless, it is reasonable to request a simple comparison showing
that the slowdown, if any, is not dramatic; such a comparison could be
performed e.g. by wrapping the invocation of the existing
dct_unquantize_intra/inter() calls in START/STOP_TIMER and comparing
the results of decoding the same file (e.g. from among the FATE
samples); such a comparison should not require more than half an hour
of Rémi's time;
* we emphasise that this does NOT imply that no slowdown is permitted,
rather that
a) in this case it should not be dramatic;
b) in general it should be judged on a case by case basis, and
always balanced out by other merits.
Furthermore, we would like to add the following comments:
* the TC should be invoked as a last resort when no other ways of
resolving the disagreement are available; it does not seem to us that
this was the case here, as it was not established e.g. what kind of a
benchmark would satisfy Andreas' concerns;
* when the TC does end up being invoked, summarizing the problem in more
detail (rather than leaving all information gathering up to TC
members) can help in speeding up the proceedings;
* while review is a critical part of the development process, excessive
amounts of reviews comments may demotivate the submitter and result in
good code never making it into git master; we would thus like to ask
all reviewers to:
a) make sure their review comments are actionable - i.e. what change
to the patch would the reviewer like to see;
b) consider the amount of work they are requesting from the
submitter and whether it is commensurate with the gains from the
change.
[1]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240609092709.1356010-2-r...@remlab.net/
[2]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240609162347.2541907-2-r...@remlab.net/
[3]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240610202322.786800-1-r...@remlab.net/
[4]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240610202322.786800-2-r...@remlab.net/
[5]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240612044723.175502-1-r...@remlab.net/
[6]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240612044723.175502-2-r...@remlab.net/
[7]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240701191328.466433-1-r...@remlab.net/
[8]
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240701191328.466433-2-r...@remlab.net/
Regards,
--
The FFmpeg Technical Committee
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
