[FFmpeg-devel] [PATCH v2 2/2] avformat: add demuxer for argonaut games' ASF format

[Zane van Iperen]

20/1/20 4:52 am, Carl Eugen Hoyos пишет:
> 
> Am So., 19. Jan. 2020 um 09:34 Uhr schrieb Zane van Iperen
> <zane at zanevaniperen.com>:
> 
>> +static int argo_asf_probe(const AVProbeData *p)
>> +{
>> +    int score;
>> +    ArgoASFFileHeader hdr;
>> +
>> +    if (p->buf_size < ASF_FILE_HEADER_SIZE)
>> +        return AVPROBE_SCORE_RETRY;
> 
> Either assert that AVPROBE_PADDING_SIZE >=
> ASF_FILE_HEADER_SIZE or (better) remove this hunk.
> 

I'd feel safer with an av_assert() than no check at the very least.
However, if there's actually no need for it and AVPROBE_PADDING_SIZE is
always guaranteed to be >= 24 then I'll remove it and leave a comment.

>> +
>> +    score = 0;
>> +    if (av_match_ext(p->filename, "asf"))
>> +        score += AVPROBE_SCORE_EXTENSION;
> 
> Remove this hunk, we only match the extension if no
> other way of recognizing a format works (tga).
> 

Done.

>> +
>> +    argo_asf_parse_file_header(&hdr, p->buf);
>> +
>> +    if(hdr.magic != ASF_TAG)
>> +        return 0;
>> +
>> +    if (argo_asf_is_known_version(&hdr))
>> +        score += 25;
> 
> Return MAX/2 + 1 in this case, feel free to ignore the other fields.
> 

Isn't that still a relatively low score?
I check the other fields so I can be more certain it's actually an ASF file.

The `hdr.chunk_offset` check was specifically suggested by Andreas.

> I wonder if "argo" isn't a better name for the format than "argo_asf".
> 

Well, there are other Argonaut formats I wanted have a crack at after
this, hence the "argo_" prefix:
- argo_brp: Container for intro videos, uses ADPCM for audio
- argo_cvg: Used for SFX on the PSX version of Croc 1.

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

Zane