[FFmpeg-devel] [PATCH] avcodec/xbmenc: Avoid snprintf() for data->hex conversion

[Marton Balint]

On Fri, 8 Mar 2024, Andreas Rheinhardt wrote:

> Marton Balint:
>> 
>> 
>> On Fri, 8 Mar 2024, Andreas Rheinhardt wrote:
>> 
>>> Andreas Rheinhardt:
>>>> Use a small LUT instead. Improves performance.
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>>> ---
>>>>  libavcodec/xbmenc.c | 21 +++++++++++++++------
>>>>  1 file changed, 15 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/libavcodec/xbmenc.c b/libavcodec/xbmenc.c
>>>> index cd8b73afa3..5231d4691d 100644
>>>> --- a/libavcodec/xbmenc.c
>>>> +++ b/libavcodec/xbmenc.c
>>>> @@ -20,11 +20,9 @@
>>>>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>>>> 02110-1301 USA
>>>>   */
>>>>
>>>> -#include "libavutil/reverse.h"
>>>>  #include "avcodec.h"
>>>>  #include "codec_internal.h"
>>>>  #include "encode.h"
>>>> -#include "mathops.h"
>>>>
>>>>  #define ANSI_MIN_READLINE 509
>>>>
>>>> @@ -57,14 +55,25 @@ static int xbm_encode_frame(AVCodecContext
>>>> *avctx, AVPacket *pkt,
>>>>      buf += snprintf(buf, 39, "static unsigned char image_bits[] =
>>>> {\n");
>>>>      for (i = 0, l = lineout; i < avctx->height; i++) {
>>>>          for (j = 0; j < linesize; j++) {
>>>> -            buf += snprintf(buf, 6, " 0x%02X", ff_reverse[*ptr++]);
>>>> +            // 0..15 bitreversed as chars
>>>> +            static const char lut[] = {
>>>> +                '0', '8', '4', 'C', '2', 'A', '6', 'E',
>>>> +                '1', '9', '5', 'D', '3', 'B', '7', 'F'
>>>> +            };
>>>> +            buf[0] = ' ';
>>>> +            buf[1] = '0';
>>>> +            buf[2] = 'x';
>>>> +            buf[3] = lut[*ptr & 0xF];
>>>> +            buf[4] = lut[*ptr >> 4];
>> 
>> Maybe you could use *buf++ = ... here as well, to avoid the next line.
>> But fine either way I guess.
>> 
>
> You mean *ptr++ to avoid the line after the next line? That would make
> the two lut accesses unsymmetric. And actually I prefer that both
> pointers are incremented side-by-side.
>

I meant this:

diff --git a/libavcodec/xbmenc.c b/libavcodec/xbmenc.c
index 5231d4691d..20f8951f93 100644
--- a/libavcodec/xbmenc.c
+++ b/libavcodec/xbmenc.c
@@ -60,12 +60,11 @@ static int xbm_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
                  '0', '8', '4', 'C', '2', 'A', '6', 'E',
                  '1', '9', '5', 'D', '3', 'B', '7', 'F'
              };
-            buf[0] = ' ';
-            buf[1] = '0';
-            buf[2] = 'x';
-            buf[3] = lut[*ptr & 0xF];
-            buf[4] = lut[*ptr >> 4];
-            buf += 5;
+            *buf++ = ' ';
+            *buf++ = '0';
+            *buf++ = 'x';
+            *buf++ = lut[*ptr & 0xF];
+            *buf++ = lut[*ptr >> 4];
              ptr++;
              if (--commas <= 0) {
                  *buf++ = '\n';


Regards,
Marton