diff mbox series

[FFmpeg-devel] avformat/avienc: Use AV_STRINGIFY for compile time constant

Message ID 20200314170845.25484-1-andreas.rheinhardt@gmail.com
State Accepted
Headers show
Series [FFmpeg-devel] avformat/avienc: Use AV_STRINGIFY for compile time constant | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Andreas Rheinhardt March 14, 2020, 5:08 p.m. UTC 
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
 libavformat/avienc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Michael Niedermayer March 14, 2020, 8:55 p.m. UTC | #1 
On Sat, Mar 14, 2020 at 06:08:45PM +0100, Andreas Rheinhardt wrote:
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/avienc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

will apply

thx

[...]
Anton Khirnov March 16, 2020, 10 a.m. UTC | #2 
Quoting Andreas Rheinhardt (2020-03-14 18:08:45)
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
>  libavformat/avienc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/avienc.c b/libavformat/avienc.c
> index 07e1c3319e..297d5b8964 100644
> --- a/libavformat/avienc.c
> +++ b/libavformat/avienc.c
> @@ -268,8 +268,8 @@ static int avi_write_header(AVFormatContext *s)
>      int padding;
>  
>      if (s->nb_streams > AVI_MAX_STREAM_COUNT) {
> -        av_log(s, AV_LOG_ERROR, "AVI does not support >%d streams\n",
> -               AVI_MAX_STREAM_COUNT);
> +        av_log(s, AV_LOG_ERROR, "AVI does not support "
> +               ">"AV_STRINGIFY(AVI_MAX_STREAM_COUNT)" streams\n");

Why? How does it make the code better? It's longer and less readable and
I'd think any decent compiler will already make that transformation.
Nicolas George March 16, 2020, 10:03 a.m. UTC | #3 
Anton Khirnov (12020-03-16):
> I'd think any decent compiler will already make that transformation.

Profile, don't speculate. gcc doesn't make that optimization for
printf().

And no compiler could do it for av_log() anyway.

Regards,
Anton Khirnov March 16, 2020, 10:25 a.m. UTC | #4 
Quoting Nicolas George (2020-03-16 11:03:41)
> Anton Khirnov (12020-03-16):
> > I'd think any decent compiler will already make that transformation.
> 
> Profile, don't speculate. gcc doesn't make that optimization for
> printf().
> 
> And no compiler could do it for av_log() anyway.

Okay, seems it doesn't. I wonder why.

But in any case, optimizing error messages for performance rather than
readability seems rather backwards.
Nicolas George March 16, 2020, 10:31 a.m. UTC | #5 
Anton Khirnov (12020-03-16):
> Okay, seems it doesn't. I wonder why.

Probably something to do with locales. Or just the benefit/costs is not
interesting.

> But in any case, optimizing error messages for performance rather than
> readability seems rather backwards.

That's a reasonable stance by itself.

Regards,
diff mbox series

Patch

diff --git a/libavformat/avienc.c b/libavformat/avienc.c
index 07e1c3319e..297d5b8964 100644
--- a/libavformat/avienc.c
+++ b/libavformat/avienc.c
@@ -268,8 +268,8 @@  static int avi_write_header(AVFormatContext *s)
     int padding;
 
     if (s->nb_streams > AVI_MAX_STREAM_COUNT) {
-        av_log(s, AV_LOG_ERROR, "AVI does not support >%d streams\n",
-               AVI_MAX_STREAM_COUNT);
+        av_log(s, AV_LOG_ERROR, "AVI does not support "
+               ">"AV_STRINGIFY(AVI_MAX_STREAM_COUNT)" streams\n");
         return AVERROR(EINVAL);
     }