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

[Mark Gaiser <markg85@gmail.com>]

On Wed, Feb 2, 2022 at 1:49 AM Timo Rothenpieler <timo@rothenpieler.org>
wrote:

> 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@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.
>

I'm fine changing it to someone that would feel more intuitive.
Do you have a suggestion to make it better?


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

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".