[FFmpeg-devel] [PATCH 1/2] fftools/textformat: fix print 64 bit integers

[FFmpeg-devel] [PATCH 1/2] fftools/textformat: fix print 64 bit integers

[Nicolas Gaullier]

Regression in ffprobe since textformat introduction
in d7a3f68feae0b1c3718f9d2671c6d41c60a40680.

Fixes #11638

Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
---
 fftools/textformat/avtextformat.c | 2 +-
 fftools/textformat/avtextformat.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c
index 14779e6f0c..14bad9022b 100644
--- a/fftools/textformat/avtextformat.c
+++ b/fftools/textformat/avtextformat.c
@@ -437,7 +437,7 @@ static char *value_string(const AVTextFormatContext *tctx, char *buf, int buf_si
 }
 
 
-void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int value, const char *unit)
+void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int64_t value, const char *unit)
 {
     char val_str[128];
     struct unit_value uv;
diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h
index cf23d93871..8316829af5 100644
--- a/fftools/textformat/avtextformat.h
+++ b/fftools/textformat/avtextformat.h
@@ -169,7 +169,7 @@ void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t va
 
 int avtext_print_string(AVTextFormatContext *tctx, const char *key, const char *val, int flags);
 
-void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int value, const char *unit);
+void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int64_t value, const char *unit);
 
 void avtext_print_rational(AVTextFormatContext *tctx, const char *key, AVRational q, char sep);
 
-- 
2.47.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/2] fftools/textformat: renamings in print_unit_int for consistency (cosmetic)

[Nicolas Gaullier]

Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
---
 fftools/ffprobe.c                 | 2 +-
 fftools/graph/graphprint.c        | 2 +-
 fftools/textformat/avtextformat.c | 4 ++--
 fftools/textformat/avtextformat.h | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
index 1346ed33c5..e8cde01407 100644
--- a/fftools/ffprobe.c
+++ b/fftools/ffprobe.c
@@ -429,7 +429,7 @@ static void log_callback(void *ptr, int level, const char *fmt, va_list vl)
 #define print_ts(k, v)          avtext_print_ts(tfc, k, v, 0)
 #define print_duration_time(k, v, tb) avtext_print_time(tfc, k, v, tb, 1)
 #define print_duration_ts(k, v)       avtext_print_ts(tfc, k, v, 1)
-#define print_val(k, v, u)      avtext_print_unit_int(tfc, k, v, u)
+#define print_val(k, v, u)      avtext_print_unit_integer(tfc, k, v, u)
 
 #define REALLOCZ_ARRAY_STREAM(ptr, cur_n, new_n)                        \
 {                                                                       \
diff --git a/fftools/graph/graphprint.c b/fftools/graph/graphprint.c
index e4c6886cf8..242eaf8ba1 100644
--- a/fftools/graph/graphprint.c
+++ b/fftools/graph/graphprint.c
@@ -140,7 +140,7 @@ typedef struct GraphPrintContext {
 #define print_q(k, v, s)        avtext_print_rational(tfc, k, v, s)
 #define print_str(k, v)         avtext_print_string(tfc, k, v, 0)
 #define print_str_opt(k, v)     avtext_print_string(tfc, k, v, gpc->opt_flags)
-#define print_val(k, v, u)      avtext_print_unit_int(tfc, k, v, u)
+#define print_val(k, v, u)      avtext_print_unit_integer(tfc, k, v, u)
 
 #define print_fmt(k, f, ...) do {              \
     av_bprint_clear(&gpc->pbuf);                    \
diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c
index 14bad9022b..396fe2f853 100644
--- a/fftools/textformat/avtextformat.c
+++ b/fftools/textformat/avtextformat.c
@@ -437,11 +437,11 @@ static char *value_string(const AVTextFormatContext *tctx, char *buf, int buf_si
 }
 
 
-void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int64_t value, const char *unit)
+void avtext_print_unit_integer(AVTextFormatContext *tctx, const char *key, int64_t val, const char *unit)
 {
     char val_str[128];
     struct unit_value uv;
-    uv.val.i = value;
+    uv.val.i = val;
     uv.unit = unit;
     avtext_print_string(tctx, key, value_string(tctx, val_str, sizeof(val_str), uv), 0);
 }
diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h
index 8316829af5..d9c14069eb 100644
--- a/fftools/textformat/avtextformat.h
+++ b/fftools/textformat/avtextformat.h
@@ -169,7 +169,7 @@ void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t va
 
 int avtext_print_string(AVTextFormatContext *tctx, const char *key, const char *val, int flags);
 
-void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int64_t value, const char *unit);
+void avtext_print_unit_integer(AVTextFormatContext *tctx, const char *key, int64_t val, const char *unit);
 
 void avtext_print_rational(AVTextFormatContext *tctx, const char *key, AVRational q, char sep);
 
-- 
2.47.2

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] fftools/textformat: fix print 64 bit integers

[Marvin Scholz]

On 20 Jun 2025, at 13:07, Nicolas Gaullier wrote:

> Regression in ffprobe since textformat introduction
> in d7a3f68feae0b1c3718f9d2671c6d41c60a40680.
>
> Fixes #11638
>
> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> ---
>  fftools/textformat/avtextformat.c | 2 +-
>  fftools/textformat/avtextformat.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c
> index 14779e6f0c..14bad9022b 100644
> --- a/fftools/textformat/avtextformat.c
> +++ b/fftools/textformat/avtextformat.c
> @@ -437,7 +437,7 @@ static char *value_string(const AVTextFormatContext *tctx, char *buf, int buf_si
>  }
>
>
> -void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int value, const char *unit)
> +void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int64_t value, const char *unit)
>  {
>      char val_str[128];
>      struct unit_value uv;
> diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h
> index cf23d93871..8316829af5 100644
> --- a/fftools/textformat/avtextformat.h
> +++ b/fftools/textformat/avtextformat.h
> @@ -169,7 +169,7 @@ void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t va
>
>  int avtext_print_string(AVTextFormatContext *tctx, const char *key, const char *val, int flags);
>
> -void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int value, const char *unit);
> +void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int64_t value, const char *unit);
>
>  void avtext_print_rational(AVTextFormatContext *tctx, const char *key, AVRational q, char sep);
>
> -- 
> 2.47.2
>

Looks good.

Would it be possible to add a test for this to make sure we catch such
a regression next time?

(I can also give it a try if you don't want to)

> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [EXTERNE] Re: [PATCH 1/2] fftools/textformat: fix print 64 bit integers

[Nicolas Gaullier]

On 6/20/25 13:29, Marvin Scholz wrote:
> On 20 Jun 2025, at 13:07, Nicolas Gaullier wrote:
>
>> Regression in ffprobe since textformat introduction
>> in d7a3f68feae0b1c3718f9d2671c6d41c60a40680.
>>
>> Fixes #11638
>>
>> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
>> ---
>>   fftools/textformat/avtextformat.c | 2 +-
>>   fftools/textformat/avtextformat.h | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fftools/textformat/avtextformat.c b/fftools/textformat/avtextformat.c
>> index 14779e6f0c..14bad9022b 100644
>> --- a/fftools/textformat/avtextformat.c
>> +++ b/fftools/textformat/avtextformat.c
>> @@ -437,7 +437,7 @@ static char *value_string(const AVTextFormatContext *tctx, char *buf, int buf_si
>>   }
>>
>>
>> -void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int value, const char *unit)
>> +void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int64_t value, const char *unit)
>>   {
>>       char val_str[128];
>>       struct unit_value uv;
>> diff --git a/fftools/textformat/avtextformat.h b/fftools/textformat/avtextformat.h
>> index cf23d93871..8316829af5 100644
>> --- a/fftools/textformat/avtextformat.h
>> +++ b/fftools/textformat/avtextformat.h
>> @@ -169,7 +169,7 @@ void avtext_print_integer(AVTextFormatContext *tctx, const char *key, int64_t va
>>
>>   int avtext_print_string(AVTextFormatContext *tctx, const char *key, const char *val, int flags);
>>
>> -void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int value, const char *unit);
>> +void avtext_print_unit_int(AVTextFormatContext *tctx, const char *key, int64_t value, const char *unit);
>>
>>   void avtext_print_rational(AVTextFormatContext *tctx, const char *key, AVRational q, char sep);
>>
>> --
>> 2.47.2
>>
> Looks good.
>
> Would it be possible to add a test for this to make sure we catch such
> a regression next time?
>
> (I can also give it a try if you don't want to)

I suppose it would be one more entry in ffprobe.mak.

First thing is this "print_unit_int"...
I found the sox format is a possibility to get a high bitrate for testing:
./ffmpeg -y -f lavfi -i "anullsrc=d=1ms:r=1234567890:cl=22.2" -f sox -
| ./ffprobe -f sox -i - -of flat -show_entries stream=bit_rate
It works, it's quick, but I am wondering if it is not a bit "too much" 
for such a very limited test.

Second thing is the "print_integer" (with no unit), which was not 
affected by the regression,
but I guess we would like to include a test for it too.
That could be done typically with some HDR metadata, I think, but this
is an additional test again...

Another way to do the job would be to make up a dedicated sample and add 
it to the fate-suite,
but that does not look very great.

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] fftools/textformat: fix print 64 bit integers

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas Gaullier
> Sent: Freitag, 20. Juni 2025 13:08
> To: ffmpeg-devel@ffmpeg.org
> Cc: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> Subject: [FFmpeg-devel] [PATCH 1/2] fftools/textformat: fix print
> 64 bit integers
> 
> Regression in ffprobe since textformat introduction
> in d7a3f68feae0b1c3718f9d2671c6d41c60a40680.
> 
> Fixes #11638
> 
> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> ---
>  fftools/textformat/avtextformat.c | 2 +-
>  fftools/textformat/avtextformat.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fftools/textformat/avtextformat.c
> b/fftools/textformat/avtextformat.c
> index 14779e6f0c..14bad9022b 100644
> --- a/fftools/textformat/avtextformat.c
> +++ b/fftools/textformat/avtextformat.c
> @@ -437,7 +437,7 @@ static char *value_string(const
> AVTextFormatContext *tctx, char *buf, int buf_si
>  }
> 
> 
> -void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int value, const char *unit)
> +void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int64_t value, const char *unit)
>  {
>      char val_str[128];
>      struct unit_value uv;
> diff --git a/fftools/textformat/avtextformat.h
> b/fftools/textformat/avtextformat.h
> index cf23d93871..8316829af5 100644
> --- a/fftools/textformat/avtextformat.h
> +++ b/fftools/textformat/avtextformat.h
> @@ -169,7 +169,7 @@ void avtext_print_integer(AVTextFormatContext
> *tctx, const char *key, int64_t va
> 
>  int avtext_print_string(AVTextFormatContext *tctx, const char
> *key, const char *val, int flags);
> 
> -void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int value, const char *unit);
> +void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int64_t value, const char *unit);
> 
>  void avtext_print_rational(AVTextFormatContext *tctx, const char
> *key, AVRational q, char sep);
> 
> --

Hi Nicolas,

LGTM!

Thanks a lot for the fix,
sw
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/2] fftools/textformat: renamings in print_unit_int for consistency (cosmetic)

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas Gaullier
> Sent: Freitag, 20. Juni 2025 13:08
> To: ffmpeg-devel@ffmpeg.org
> Cc: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> Subject: [FFmpeg-devel] [PATCH 2/2] fftools/textformat: renamings
> in print_unit_int for consistency (cosmetic)
> 
> Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> ---
>  fftools/ffprobe.c                 | 2 +-
>  fftools/graph/graphprint.c        | 2 +-
>  fftools/textformat/avtextformat.c | 4 ++--
>  fftools/textformat/avtextformat.h | 2 +-
>  4 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/fftools/ffprobe.c b/fftools/ffprobe.c
> index 1346ed33c5..e8cde01407 100644
> --- a/fftools/ffprobe.c
> +++ b/fftools/ffprobe.c
> @@ -429,7 +429,7 @@ static void log_callback(void *ptr, int level,
> const char *fmt, va_list vl)
>  #define print_ts(k, v)          avtext_print_ts(tfc, k, v, 0)
>  #define print_duration_time(k, v, tb) avtext_print_time(tfc, k, v,
> tb, 1)
>  #define print_duration_ts(k, v)       avtext_print_ts(tfc, k, v,
> 1)
> -#define print_val(k, v, u)      avtext_print_unit_int(tfc, k, v,
> u)
> +#define print_val(k, v, u)      avtext_print_unit_integer(tfc, k,
> v, u)
> 
>  #define REALLOCZ_ARRAY_STREAM(ptr, cur_n, new_n)
> \
>  {
> \
> diff --git a/fftools/graph/graphprint.c
> b/fftools/graph/graphprint.c
> index e4c6886cf8..242eaf8ba1 100644
> --- a/fftools/graph/graphprint.c
> +++ b/fftools/graph/graphprint.c
> @@ -140,7 +140,7 @@ typedef struct GraphPrintContext {
>  #define print_q(k, v, s)        avtext_print_rational(tfc, k, v,
> s)
>  #define print_str(k, v)         avtext_print_string(tfc, k, v, 0)
>  #define print_str_opt(k, v)     avtext_print_string(tfc, k, v,
> gpc->opt_flags)
> -#define print_val(k, v, u)      avtext_print_unit_int(tfc, k, v,
> u)
> +#define print_val(k, v, u)      avtext_print_unit_integer(tfc, k,
> v, u)
> 
>  #define print_fmt(k, f, ...) do {              \
>      av_bprint_clear(&gpc->pbuf);                    \
> diff --git a/fftools/textformat/avtextformat.c
> b/fftools/textformat/avtextformat.c
> index 14bad9022b..396fe2f853 100644
> --- a/fftools/textformat/avtextformat.c
> +++ b/fftools/textformat/avtextformat.c
> @@ -437,11 +437,11 @@ static char *value_string(const
> AVTextFormatContext *tctx, char *buf, int buf_si
>  }
> 
> 
> -void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int64_t value, const char *unit)
> +void avtext_print_unit_integer(AVTextFormatContext *tctx, const
> char *key, int64_t val, const char *unit)
>  {
>      char val_str[128];
>      struct unit_value uv;
> -    uv.val.i = value;
> +    uv.val.i = val;
>      uv.unit = unit;
>      avtext_print_string(tctx, key, value_string(tctx, val_str,
> sizeof(val_str), uv), 0);
>  }
> diff --git a/fftools/textformat/avtextformat.h
> b/fftools/textformat/avtextformat.h
> index 8316829af5..d9c14069eb 100644
> --- a/fftools/textformat/avtextformat.h
> +++ b/fftools/textformat/avtextformat.h
> @@ -169,7 +169,7 @@ void avtext_print_integer(AVTextFormatContext
> *tctx, const char *key, int64_t va
> 
>  int avtext_print_string(AVTextFormatContext *tctx, const char
> *key, const char *val, int flags);
> 
> -void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int64_t value, const char *unit);
> +void avtext_print_unit_integer(AVTextFormatContext *tctx, const
> char *key, int64_t val, const char *unit);
> 
>  void avtext_print_rational(AVTextFormatContext *tctx, const char
> *key, AVRational q, char sep);
> 
> --

Hi Nicolas,

thanks for the patch. Makes sense and 
LGTM.

softworkz

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/2] fftools/textformat: fix print 64 bit integers

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Marvin
> Scholz
> Sent: Freitag, 20. Juni 2025 13:30
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Cc: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] fftools/textformat: fix print 64
> bit integers
> 
> 
> 
> On 20 Jun 2025, at 13:07, Nicolas Gaullier wrote:
> 
> > Regression in ffprobe since textformat introduction
> > in d7a3f68feae0b1c3718f9d2671c6d41c60a40680.
> >
> > Fixes #11638
> >
> > Signed-off-by: Nicolas Gaullier <nicolas.gaullier@cji.paris>
> > ---
> >  fftools/textformat/avtextformat.c | 2 +-
> >  fftools/textformat/avtextformat.h | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fftools/textformat/avtextformat.c
> b/fftools/textformat/avtextformat.c
> > index 14779e6f0c..14bad9022b 100644
> > --- a/fftools/textformat/avtextformat.c
> > +++ b/fftools/textformat/avtextformat.c
> > @@ -437,7 +437,7 @@ static char *value_string(const
> AVTextFormatContext *tctx, char *buf, int buf_si
> >  }
> >
> >
> > -void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int value, const char *unit)
> > +void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int64_t value, const char *unit)
> >  {
> >      char val_str[128];
> >      struct unit_value uv;
> > diff --git a/fftools/textformat/avtextformat.h
> b/fftools/textformat/avtextformat.h
> > index cf23d93871..8316829af5 100644
> > --- a/fftools/textformat/avtextformat.h
> > +++ b/fftools/textformat/avtextformat.h
> > @@ -169,7 +169,7 @@ void avtext_print_integer(AVTextFormatContext
> *tctx, const char *key, int64_t va
> >
> >  int avtext_print_string(AVTextFormatContext *tctx, const char *key,
> const char *val, int flags);
> >
> > -void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int value, const char *unit);
> > +void avtext_print_unit_int(AVTextFormatContext *tctx, const char
> *key, int64_t value, const char *unit);
> >
> >  void avtext_print_rational(AVTextFormatContext *tctx, const char
> *key, AVRational q, char sep);
> >
> > --
> > 2.47.2
> >
> 
> Looks good.
> 
> Would it be possible to add a test for this to make sure we catch such
> a regression next time?
> 
> (I can also give it a try if you don't want to)

Hi Marvin and Nicolas,

tests are always good. In this case though, I'm wondering whether it
might be better to cleanup and straighten the APIs and implementations
first. 

(look at the value_string() function to see what I mean).

Best regards,
sw




_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".