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

[Zane van Iperen]

22/1/20 9:52 am, Carl Eugen Hoyos пишет:
> 
> Am Mi., 22. Jan. 2020 um 00:33 Uhr schrieb Zane van Iperen
> <zane at zanevaniperen.com>:
>>
>> 21/1/20 11:14 pm, Carl Eugen Hoyos пишет:
>>>
>>> Am Di., 21. Jan. 2020 um 11:09 Uhr schrieb Zane van Iperen
>>> <zane at zanevaniperen.com>:
>>>
>>>> +static int argo_asf_probe(const AVProbeData *p)
>>>> +{
>>>> +    int score;
>>>> +    ArgoASFFileHeader hdr;
>>>> +
>>>> +    av_assert0(AVPROBE_PADDING_SIZE >= ASF_FILE_HEADER_SIZE);
>>>> +
>>>> +    argo_asf_parse_file_header(&hdr, p->buf);
>>>> +
>>>> +    if (hdr.magic != ASF_TAG)
>>>> +        return 0;
>>>
>>>> +    /* If this is huge, then it's very likely not an ASF file. */
>>>> +    if (hdr.chunk_offset > INT_MAX)
>>>> +        return 1;
>>>
>>> Am I correct that this tests only one bit?
>>> This makes sense for many probe functions that can easily
>>> return false positives based on weak heuristics, probe
>>> functions typically return high scores for matching first 32
>>> bits though.
>>
>> I can just get rid of that check completely if it's easier.
> 
> Please.
> 

Done.

>>>> +    score = 0;
>>>> +    if (argo_asf_is_known_version(&hdr))
>>>> +        score += (AVPROBE_SCORE_MAX / 2) + 1;
>>>
>>> Unusual code style.
>>>
>>>> +    /* Have only ever seen these with 1 chunk. */
>>>> +    if (hdr.num_chunks == 1)
>>>> +        score += 10;
>>>
>>> Returns 0 for argo files with unknown version and more than
>>> one chunk and 10 for for unknown version and one chunk.
>>>
>>> Should be ~25 in both cases, feel free to return a higher
>>> score if all tests pass, I just wanted to simplify the probe
>>> function as testing 32 bit is what is usually done.
>>>
>>
>> Returning 0 on a unknown version is a bug, I want it to hit the
>> avpriv_request_sample() below.
>>
>> Would something like this (untested) be better?
>>
>>   >     if (hdr.chunk_offset & 0xFF000000U)
>>   >         return 1;
>>   >
>>   >     score = (AVPROBE_SCORE_MAX / 2) + 1;
>>   >
>>   >     if (!argo_asf_is_known_version(&hdr))
>>   >         score -= 25;
>>   >
>>   >     /* Have only ever seen these with 1 chunk. */
>>   >     if (hdr.num_chunks > 1)
>>   >         score -= 25;
>>   >
>>   >     return score;
>>
>> That way if it fails one check it'll be 26, or 1 if it fails both.
> 
> But it should return ~25 if the first 32bit match (but the version
> is wrong) and fail later.
> 
> I mostly wanted to argue for a less complicated probe function
> for a format that starts with 32 known bits.
> 

Ah, sorry I misunderstood. How's this?

 >    if (hdr.magic != ASF_TAG)
 >        return 0;
 >
 >    if (!argo_asf_is_known_version(&hdr))
 >        return 25;
 >
 >    return (AVPROBE_SCORE_MAX / 2) + 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