[FFmpeg-devel] [PATCH 4/6] avformat/argo_asf: add version_major and version_minor options

[Alexander Strasser]

Am 8. August 2020 14:52:02 MESZ schrieb Zane van Iperen <zane at zanevaniperen.com>:
>On Sat, 08 Aug 2020 13:05:28 +0200
>"Alexander Strasser" <eclipse7 at gmx.net> wrote:
>
>> >@@ -296,8 +298,7 @@ static int argo_asf_write_header(AVFormatContext
>> >*s)
>> >     ArgoASFContext          *ctx = s->priv_data;
>> >
>> >     ctx->fhdr.magic              = ASF_TAG;
>> >-    ctx->fhdr.version_major      = 2;
>> >-    ctx->fhdr.version_minor      = 1;
>> >+    /* version_{major,minor} set by options. */
>> 
>> Stupid question: Why are you adding options to override the file
>format version?
>> 
>> Normally those versions have implications on  the syntax and/or
>semantics of the files they are embedded in.
>> 
>> Am I misreading or misunderstanding something?
>> 
>
>You're right to think that, that's what I originally thought too.
>When researching the file format, I investigated several different file
>versions and their structure was identical.
>
>I think it's really only used as some kind of psuedo-identifier that
>tells which game the file came from, and I'm pretty sure some of the
>games check the version too (hence the options).
>
>I'd be happy to be proven wrong, but until other files surface I think
>allowing the user to specify the version is fine.

I see. Maybe it's not the version of the format, but has implications of what can be inside. Anyway I didn't look at any files, so just some wild guessing.

I would advise to use the verb override instead of set in the option description. I think it's less ambiguous. Like more expectations "you are doing this on your own" and less "FFmpeg will use Version 2.4 of the format now".


  Alexander