[FFmpeg-devel] [PATCH 5/6] avformat/argo_asf: add name option

[Zane van Iperen]

On Sat, 8 Aug 2020 12:20:32 +0200
"Andreas Rheinhardt" <andreas.rheinhardt at gmail.com> wrote:

> >  #if CONFIG_ARGO_ASF_DEMUXER
> > @@ -301,7 +302,10 @@ static int argo_asf_write_header(AVFormatContext *s)
> >      /* version_{major,minor} set by options. */
> >      ctx->fhdr.num_chunks         = 1;
> >      ctx->fhdr.chunk_offset       = ASF_FILE_HEADER_SIZE;
> > -    strncpy(ctx->fhdr.name, av_basename(s->url), FF_ARRAY_ELEMS(ctx->fhdr.name));
> > +    if (ctx->name)
> > +        strncpy(ctx->fhdr.name, ctx->name, FF_ARRAY_ELEMS(ctx->fhdr.name));
> > +    else
> > +        strncpy(ctx->fhdr.name, av_basename(s->url), FF_ARRAY_ELEMS(ctx->fhdr.name));
> 
> FF_ARRAY_ELEMS() is actually inappropriate here: Use sizeof() directly.
Fixed.

> No need to make a separate patch for it.
Do you mean squash it into the previous one where I add the version
options?

> >
> >      ctx->ckhdr.num_blocks        = 0;
> >      ctx->ckhdr.num_samples       = ASF_SAMPLE_COUNT;
> > @@ -362,6 +366,14 @@ static const AVOption argo_asf_options[] = {
> >          .max         = UINT16_MAX,
> >          .flags       = AV_OPT_FLAG_ENCODING_PARAM
> >      },
> > +    {
> > +        .name        = "name",
> > +        .help        = "embedded file name (max 8 characters)",
> > +        .offset      = offsetof(ArgoASFContext, name),
> > +        .type        = AV_OPT_TYPE_STRING,
> > +        .default_val = {.str = NULL},
> > +        .flags       = AV_OPT_FLAG_ENCODING_PARAM
> > +    },
> >      { NULL }
> >  };
> I wonder whether it would not be better to set this via metadata (that
> the demuxer could export so that it survives a roundtrip).

Hmmm, possibly? I've only ever seen the field as the filename (minus
extension), so I'm not too sure how much value it has as metadata.

Zane