[FFmpeg-devel] [PATCH v2 1/1] avformat: Add IPFS protocol support.

[Andreas Rheinhardt]

Timo Rothenpieler:
> On 02.02.2022 01:33, Mark Gaiser wrote:
>> On Wed, Feb 2, 2022 at 1:27 AM Timo Rothenpieler <timo at rothenpieler.org>
>> wrote:
>>
>>> On 01.02.2022 22:58, Mark Gaiser wrote:
>>>> +static int translate_ipfs_to_http(URLContext *h, const char *uri, int
>>> flags, AVDictionary **options)
>>>> +{
>>>> +    const char *ipfs_cid;
>>>> +    const char *protocol_path_suffix = "ipfs/";
>>>> +    char *fulluri;
>>>> +    int ret;
>>>> +    Context *c = h->priv_data;
>>>> +    int is_ipfs = (av_strstart(uri, "ipfs://", &ipfs_cid) ||
>>> av_strstart(uri, "ipfs:", &ipfs_cid));
>>>> +    int is_ipns = (av_strstart(uri, "ipns://", &ipfs_cid) ||
>>> av_strstart(uri, "ipns:", &ipfs_cid));
>>>
>>> What's the point of this logic?
>>> The first half of each check seems pointless, since the second half is
>>> true for everything the first one would cover.
>>>
>>
>> Hi Time,
>>
>> The point it to allow
>> ipfs://<cid> and ipfs:<cid>
>>
>> So for that i want to test for all possible true situations (ipfs://,
>> ipfs:, ipns:// and ipns:).
> 
> If the url starts with "ipns://", it obviously also starts with "ipns:",
> so checking for the longer of the two is pointless.
> Same for "ipfs:".

You forgot that av_strstart() also sets ipfs_cid which is used later;
the above code exists to strip the leading protocol part away.

> 
>> This is akin to other protocols who seem to do the same check. Look at
>> crypto.c for example.
>> Another point is further down where the url is composed.
>> If it's ipfs it becomes a url like <gateway>/ipfs
>> And for ipns: <gateway>/ipns
>