[FFmpeg-devel] [TC] Decision on H.263 DCT dequantization

[FFmpeg Technical Committee]

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-remi@remlab.net/
[2] https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240609162347.2541907-2-remi@remlab.net/
[3] https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240610202322.786800-1-remi@remlab.net/
[4] https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240610202322.786800-2-remi@remlab.net/
[5] https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240612044723.175502-1-remi@remlab.net/
[6] https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240612044723.175502-2-remi@remlab.net/
[7] https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240701191328.466433-1-remi@remlab.net/
[8] https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240701191328.466433-2-remi@remlab.net/

Regards,
-- 
The FFmpeg Technical Committee