diff mbox series

[FFmpeg-devel,v3,5/5] avformat/argo_asf: strip file extension from name

Message ID 20200809234441.2365933-5-zane@zanevaniperen.com
State Accepted
Headers show
Series [FFmpeg-devel,v3,1/5] avformat/argo_asf: add games to version list
Related show

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Zane van Iperen Aug. 9, 2020, 11:45 p.m. UTC
Only when the user hasn't manually specified one.
Matches the original files more closely.

Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
---
 libavformat/argo_asf.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

Comments

Alexander Strasser Aug. 17, 2020, 8:14 a.m. UTC | #1
Hi Zane,

sorry for this late remark.

On 2020-08-09 23:45 +0000, Zane van Iperen wrote:
> Only when the user hasn't manually specified one.
> Matches the original files more closely.
>
> Signed-off-by: Zane van Iperen <zane@zanevaniperen.com>
> ---
>  libavformat/argo_asf.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c
> index 577b9d9c01..37ad2bf5e9 100644
> --- a/libavformat/argo_asf.c
> +++ b/libavformat/argo_asf.c
> @@ -329,10 +329,24 @@ static int argo_asf_write_header(AVFormatContext *s)
>      fhdr.version_minor = (uint16_t)ctx->version_minor;
>      fhdr.num_chunks    = 1;
>      fhdr.chunk_offset  = ASF_FILE_HEADER_SIZE;
> -    if (ctx->name)
> +    /*
> +     * If the user specified a name, use it as is. Otherwise take the
> +     * basename and lop off the extension (if any).
> +     */
> +    if (ctx->name) {
>          strncpy(fhdr.name, ctx->name, sizeof(fhdr.name));
> -    else
> -        strncpy(fhdr.name, av_basename(s->url), sizeof(fhdr.name));
> +    } else {
> +        const char *start = av_basename(s->url);
> +        const char *end   = strrchr(start, '.');
> +        size_t      len;
> +
> +        if(end)
> +            len = end - start;
> +        else
> +            len = strlen(start);
> +
> +        memcpy(fhdr.name, start, FFMIN(len, sizeof(fhdr.name)));
> +    }

How is one supposed to make sense of the name field when reading it?

It's declared on the stack further above in this function:

    ArgoASFFileHeader  fhdr;

And I can spot no further intitialization or a count field.
Later there is a call

    argo_asf_write_file_header(&fhdr, s->pb);

So I guess my question is: should remaning bytes be zeroed like in the
if clause?

If yes, then that's missing from the else clause.

And the second question is: If a full size name is written, is it
allowed to have no zero termination?


[...]

  Alexander
diff mbox series

Patch

diff --git a/libavformat/argo_asf.c b/libavformat/argo_asf.c
index 577b9d9c01..37ad2bf5e9 100644
--- a/libavformat/argo_asf.c
+++ b/libavformat/argo_asf.c
@@ -329,10 +329,24 @@  static int argo_asf_write_header(AVFormatContext *s)
     fhdr.version_minor = (uint16_t)ctx->version_minor;
     fhdr.num_chunks    = 1;
     fhdr.chunk_offset  = ASF_FILE_HEADER_SIZE;
-    if (ctx->name)
+    /*
+     * If the user specified a name, use it as is. Otherwise take the
+     * basename and lop off the extension (if any).
+     */
+    if (ctx->name) {
         strncpy(fhdr.name, ctx->name, sizeof(fhdr.name));
-    else
-        strncpy(fhdr.name, av_basename(s->url), sizeof(fhdr.name));
+    } else {
+        const char *start = av_basename(s->url);
+        const char *end   = strrchr(start, '.');
+        size_t      len;
+
+        if(end)
+            len = end - start;
+        else
+            len = strlen(start);
+
+        memcpy(fhdr.name, start, FFMIN(len, sizeof(fhdr.name)));
+    }
 
     chdr.num_blocks    = 0;
     chdr.num_samples   = ASF_SAMPLE_COUNT;