diff mbox series

[FFmpeg-devel,1/2] avformat/argo_cvg: name unk{1, 2} fields correctly

Message ID 20220717133143.881124-1-zane@zanevaniperen.com
State New
Headers show
Series [FFmpeg-devel,1/2] avformat/argo_cvg: name unk{1, 2} fields correctly | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Zane van Iperen July 17, 2022, 1:31 p.m. UTC
Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
---
 libavformat/argo_cvg.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

Comments

Andreas Rheinhardt July 18, 2022, 1:45 p.m. UTC | #1
Zane van Iperen:
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>  libavformat/argo_cvg.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/libavformat/argo_cvg.c b/libavformat/argo_cvg.c
> index f32487023a..2ee4a64449 100644
> --- a/libavformat/argo_cvg.c
> +++ b/libavformat/argo_cvg.c
> @@ -40,9 +40,9 @@
>  #define ARGO_CVG_SAMPLES_PER_BLOCK  28
>  
>  typedef struct ArgoCVGHeader {
> -    uint32_t size; /*< File size -8 (this + trailing checksum) */
> -    uint32_t unk1; /*< Unknown. Always seems to be 0 or 1. */
> -    uint32_t unk2; /*< Unknown. Always seems to be 0 or 1. */
> +    uint32_t size;   /*< File size -8 (this + trailing checksum) */
> +    uint32_t loop;   /*< Loop flag. */
> +    uint32_t reverb; /*< Reverb flag. */
>  } ArgoCVGHeader;
>  
>  typedef struct ArgoCVGOverride {
> @@ -91,17 +91,17 @@ static int argo_cvg_probe(const AVProbeData *p)
>      if (p->buf_size < ARGO_CVG_HEADER_SIZE)
>          return 0;
>  
> -    cvg.size = AV_RL32(p->buf + 0);
> -    cvg.unk1 = AV_RL32(p->buf + 4);
> -    cvg.unk2 = AV_RL32(p->buf + 8);
> +    cvg.size   = AV_RL32(p->buf + 0);
> +    cvg.loop   = AV_RL32(p->buf + 4);
> +    cvg.reverb = AV_RL32(p->buf + 8);
>  
>      if (cvg.size < 8)
>          return 0;
>  
> -    if (cvg.unk1 != 0 && cvg.unk1 != 1)
> +    if (cvg.loop != 0 && cvg.loop != 1)
>          return 0;
>  
> -    if (cvg.unk2 != 0 && cvg.unk2 != 1)
> +    if (cvg.reverb != 0 && cvg.reverb != 1)
>          return 0;
>  
>      return AVPROBE_SCORE_MAX / 4 + 1;
> @@ -150,15 +150,16 @@ static int argo_cvg_read_header(AVFormatContext *s)
>      else if (ret != ARGO_CVG_HEADER_SIZE)
>          return AVERROR(EIO);
>  
> -    ctx->header.size = AV_RL32(buf + 0);
> -    ctx->header.unk1 = AV_RL32(buf + 4);
> -    ctx->header.unk2 = AV_RL32(buf + 8);
> +    ctx->header.size   = AV_RL32(buf + 0);
> +    ctx->header.loop   = AV_RL32(buf + 4);
> +    ctx->header.reverb = AV_RL32(buf + 8);
>  
>      if (ctx->header.size < 8)
>          return AVERROR_INVALIDDATA;
>  
>      av_log(s, AV_LOG_TRACE, "size       = %u\n", ctx->header.size);
> -    av_log(s, AV_LOG_TRACE, "unk        = %u, %u\n", ctx->header.unk1, ctx->header.unk2);
> +    av_log(s, AV_LOG_TRACE, "loop       = %u\n", ctx->header.loop);
> +    av_log(s, AV_LOG_TRACE, "reverb     = %u\n", ctx->header.reverb);

%u is for unsigned, yet these variables are of type uint32_t. It is not
guaranteed that these two types are the same (yet they typically are).
That's why the PRI-macros exist.
(Apart from that: Does this have to be three separate av_logs?)

>  
>      if ((ret = argo_cvg_read_checksum(s->pb, &ctx->header, &ctx->checksum)) < 0)
>          return ret;
> @@ -172,10 +173,10 @@ static int argo_cvg_read_header(AVFormatContext *s)
>  
>      for (size_t i = 0; i < FF_ARRAY_ELEMS(overrides); i++) {
>          const ArgoCVGOverride *ovr = overrides + i;
> -        if (ovr->header.size != ctx->header.size ||
> -            ovr->header.unk1 != ctx->header.unk1 ||
> -            ovr->header.unk2 != ctx->header.unk2 ||
> -            ovr->checksum    != ctx->checksum    ||
> +        if (ovr->header.size   != ctx->header.size ||
> +            ovr->header.loop   != ctx->header.loop ||
> +            ovr->header.reverb != ctx->header.reverb ||
> +            ovr->checksum      != ctx->checksum    ||
>              av_strcasecmp(filename, ovr->name) != 0)
>              continue;
>
Zane van Iperen July 20, 2022, 8:56 a.m. UTC | #2
On 18/7/2022 11:45 pm, Andreas Rheinhardt wrote:

>>       av_log(s, AV_LOG_TRACE, "size       = %u\n", ctx->header.size);
>> -    av_log(s, AV_LOG_TRACE, "unk        = %u, %u\n", ctx->header.unk1, ctx->header.unk2);
>> +    av_log(s, AV_LOG_TRACE, "loop       = %u\n", ctx->header.loop);
>> +    av_log(s, AV_LOG_TRACE, "reverb     = %u\n", ctx->header.reverb);
> 
> %u is for unsigned, yet these variables are of type uint32_t. It is not
> guaranteed that these two types are the same (yet they typically are).
> That's why the PRI-macros exist.

Good catch, I'll rectify that.

> (Apart from that: Does this have to be three separate av_logs?)
> 
I can probably just remove them, now that I know what the fields are.

Although, I wouldn't mind exposing them as additional metadata...
diff mbox series

Patch

diff --git a/libavformat/argo_cvg.c b/libavformat/argo_cvg.c
index f32487023a..2ee4a64449 100644
--- a/libavformat/argo_cvg.c
+++ b/libavformat/argo_cvg.c
@@ -40,9 +40,9 @@ 
 #define ARGO_CVG_SAMPLES_PER_BLOCK  28
 
 typedef struct ArgoCVGHeader {
-    uint32_t size; /*< File size -8 (this + trailing checksum) */
-    uint32_t unk1; /*< Unknown. Always seems to be 0 or 1. */
-    uint32_t unk2; /*< Unknown. Always seems to be 0 or 1. */
+    uint32_t size;   /*< File size -8 (this + trailing checksum) */
+    uint32_t loop;   /*< Loop flag. */
+    uint32_t reverb; /*< Reverb flag. */
 } ArgoCVGHeader;
 
 typedef struct ArgoCVGOverride {
@@ -91,17 +91,17 @@  static int argo_cvg_probe(const AVProbeData *p)
     if (p->buf_size < ARGO_CVG_HEADER_SIZE)
         return 0;
 
-    cvg.size = AV_RL32(p->buf + 0);
-    cvg.unk1 = AV_RL32(p->buf + 4);
-    cvg.unk2 = AV_RL32(p->buf + 8);
+    cvg.size   = AV_RL32(p->buf + 0);
+    cvg.loop   = AV_RL32(p->buf + 4);
+    cvg.reverb = AV_RL32(p->buf + 8);
 
     if (cvg.size < 8)
         return 0;
 
-    if (cvg.unk1 != 0 && cvg.unk1 != 1)
+    if (cvg.loop != 0 && cvg.loop != 1)
         return 0;
 
-    if (cvg.unk2 != 0 && cvg.unk2 != 1)
+    if (cvg.reverb != 0 && cvg.reverb != 1)
         return 0;
 
     return AVPROBE_SCORE_MAX / 4 + 1;
@@ -150,15 +150,16 @@  static int argo_cvg_read_header(AVFormatContext *s)
     else if (ret != ARGO_CVG_HEADER_SIZE)
         return AVERROR(EIO);
 
-    ctx->header.size = AV_RL32(buf + 0);
-    ctx->header.unk1 = AV_RL32(buf + 4);
-    ctx->header.unk2 = AV_RL32(buf + 8);
+    ctx->header.size   = AV_RL32(buf + 0);
+    ctx->header.loop   = AV_RL32(buf + 4);
+    ctx->header.reverb = AV_RL32(buf + 8);
 
     if (ctx->header.size < 8)
         return AVERROR_INVALIDDATA;
 
     av_log(s, AV_LOG_TRACE, "size       = %u\n", ctx->header.size);
-    av_log(s, AV_LOG_TRACE, "unk        = %u, %u\n", ctx->header.unk1, ctx->header.unk2);
+    av_log(s, AV_LOG_TRACE, "loop       = %u\n", ctx->header.loop);
+    av_log(s, AV_LOG_TRACE, "reverb     = %u\n", ctx->header.reverb);
 
     if ((ret = argo_cvg_read_checksum(s->pb, &ctx->header, &ctx->checksum)) < 0)
         return ret;
@@ -172,10 +173,10 @@  static int argo_cvg_read_header(AVFormatContext *s)
 
     for (size_t i = 0; i < FF_ARRAY_ELEMS(overrides); i++) {
         const ArgoCVGOverride *ovr = overrides + i;
-        if (ovr->header.size != ctx->header.size ||
-            ovr->header.unk1 != ctx->header.unk1 ||
-            ovr->header.unk2 != ctx->header.unk2 ||
-            ovr->checksum    != ctx->checksum    ||
+        if (ovr->header.size   != ctx->header.size ||
+            ovr->header.loop   != ctx->header.loop ||
+            ovr->header.reverb != ctx->header.reverb ||
+            ovr->checksum      != ctx->checksum    ||
             av_strcasecmp(filename, ovr->name) != 0)
             continue;