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

[Timo Rothenpieler]

On 02.02.2022 01:44, Andreas Rheinhardt wrote:
> 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.

Ah, yeah. That's a very confusing side effect there.
I guess it's fine, but I can't say I like using it like that.
Since there's a large risk someone else might in the future think the 
same and break it.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4494 bytes
Desc: S/MIME Cryptographic Signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20220202/c5fbfa9c/attachment.bin>