[FFmpeg-devel] [PATCH 2/3] lavc: replace ff_thread_get_buffer() with ff_get_buffer()

[Andreas Rheinhardt]

Anton Khirnov:
> Quoting Andreas Rheinhardt (2024-03-08 11:44:12)
>> Anton Khirnov:
>>> Quoting Andreas Rheinhardt (2024-03-08 11:19:59)
>>>> Anton Khirnov:
>>>>> Quoting Andreas Rheinhardt (2024-03-08 11:03:18)
>>>>>> Anton Khirnov:
>>>>>>> ff_thread_get_buffer() has exactly the same semantics as
>>>>>>> ff_get_buffer(), except it is supposed to be used in frame-threaded
>>>>>>> decoders. Since the decoder instance knows whether frame threading is in
>>>>>>> use, there is no point in forcing decoder implementations to use a
>>>>>>> different function merely because they happen to support frame
>>>>>>> threading.
>>>>>>> ---
>>>>>>>  libavcodec/aic.c              |  4 ++--
>>>>>>>  libavcodec/alac.c             |  4 ++--
>>>>>>>  libavcodec/av1dec.c           |  2 +-
>>>>>>>  libavcodec/avcodec_internal.h | 13 +++++++++++++
>>>>>>>  libavcodec/bitpacked_dec.c    |  4 ++--
>>>>>>>  libavcodec/cfhd.c             |  4 ++--
>>>>>>>  libavcodec/cllc.c             |  8 ++++----
>>>>>>>  libavcodec/cri.c              |  2 +-
>>>>>>>  libavcodec/decode.c           | 13 ++++++++++++-
>>>>>>>  libavcodec/dnxhddec.c         |  2 +-
>>>>>>>  libavcodec/dvdec.c            |  2 +-
>>>>>>>  libavcodec/dxtory.c           | 12 ++++++------
>>>>>>>  libavcodec/dxv.c              |  4 ++--
>>>>>>>  libavcodec/exr.c              |  2 +-
>>>>>>>  libavcodec/flacdec.c          |  4 ++--
>>>>>>>  libavcodec/fraps.c            |  4 ++--
>>>>>>>  libavcodec/h264_slice.c       |  2 +-
>>>>>>>  libavcodec/hapdec.c           |  4 ++--
>>>>>>>  libavcodec/hdrdec.c           |  2 +-
>>>>>>>  libavcodec/hevcdec.c          |  2 +-
>>>>>>>  libavcodec/hqx.c              |  4 ++--
>>>>>>>  libavcodec/huffyuvdec.c       |  4 ++--
>>>>>>>  libavcodec/jpeg2000dec.c      |  2 +-
>>>>>>>  libavcodec/lagarith.c         | 12 ++++++------
>>>>>>>  libavcodec/lcldec.c           |  4 ++--
>>>>>>>  libavcodec/magicyuv.c         |  2 +-
>>>>>>>  libavcodec/mdec.c             |  4 ++--
>>>>>>>  libavcodec/notchlc.c          |  2 +-
>>>>>>>  libavcodec/photocd.c          |  2 +-
>>>>>>>  libavcodec/pixlet.c           |  2 +-
>>>>>>>  libavcodec/pngdec.c           |  4 ++--
>>>>>>>  libavcodec/proresdec2.c       |  2 +-
>>>>>>>  libavcodec/pthread_frame.c    |  7 ++-----
>>>>>>>  libavcodec/qoidec.c           |  2 +-
>>>>>>>  libavcodec/rtv1.c             |  2 +-
>>>>>>>  libavcodec/sheervideo.c       |  4 ++--
>>>>>>>  libavcodec/takdec.c           |  5 +++--
>>>>>>>  libavcodec/thread.h           | 10 ----------
>>>>>>>  libavcodec/tiff.c             |  2 +-
>>>>>>>  libavcodec/tta.c              |  4 ++--
>>>>>>>  libavcodec/utils.c            |  5 -----
>>>>>>>  libavcodec/utvideodec.c       |  4 ++--
>>>>>>>  libavcodec/v210dec.c          |  4 ++--
>>>>>>>  libavcodec/v410dec.c          |  4 ++--
>>>>>>>  libavcodec/vaapi_av1.c        |  4 ++--
>>>>>>>  libavcodec/vble.c             |  4 ++--
>>>>>>>  libavcodec/vmixdec.c          |  2 +-
>>>>>>>  libavcodec/vvc/vvc_refs.c     |  4 ++--
>>>>>>>  libavcodec/wbmpdec.c          |  2 +-
>>>>>>>  libavcodec/webp.c             |  2 +-
>>>>>>>  libavcodec/ylc.c              |  4 ++--
>>>>>>>  51 files changed, 110 insertions(+), 103 deletions(-)
>>>>>>>
>>>>>>
>>>>>> -1: This adds avoidable runtime checks.
>>>>>
>>>>> What checks and why is that a problem?
>>>>>
>>>> It adds a runtime check to every call to ff_get_buffer() by every
>>>> decoder not supporting frame-threading (by checking whether
>>>> frame-threading is currently in use).
>>>
>>> I cannot imagine any situation where it could have a measurable impact.
>>>
>>
>> And? It is avoidable, therefore it should be avoided.
> 
> Why should it be avoided when it can never have any measurable impact?
> 
> The maintenance cost of a more complex API is higher than the
> infinitesimal cost of this check.
> 

What maintenance cost and complexity are you referring to? I checked and
could not find a single commit where one had to fix an ff_get_buffer()
to ff_thread_get_buffer() because it has been forgotten when the decoder
has been declared to support frame threading.

- Andreas