On Fri, Feb 4, 2022 at 11:29 AM Tomas Härdin <tjop...@acc.umu.se> wrote:
> ons 2022-02-02 klockan 14:56 +0100 skrev Mark Gaiser: > > On Wed, Feb 2, 2022 at 2:21 PM Tomas Härdin <tjop...@acc.umu.se> > > wrote: > > > > > tis 2022-02-01 klockan 22:58 +0100 skrev Mark Gaiser: > > > > > > > > > > > +typedef struct Context { > > > > + AVClass *class; > > > > + URLContext *inner; > > > > + char *gateway; > > > > > > Is there not a maximum length that an HTTP URL can be? At least > > > without > > > query parameters. That way you avoid dynamic allocations. You'd > > > have to > > > separate the AVOption from such a buffer in that case, but I think > > > you > > > have to anyway. > > > > > > > Could you provide more information on that? Or an example of what you > > mean > > exactly? > > As far as i know there is no hard limit though it's very much advised > > to > > not go above 2048 characters. > > You could at least separate the variable that is set by the AVOption > stuff from the buffer that's produced by the URL parsing stuff. > > Since you call ffurl_open_whitelist() immediately you could just use > alloca() to allocate memory on the stack. That way you don't have to > worry about deallocation at all *and* you can have it be arbitrarily > large. > > If URLs have a spec'd maximum length then a buffer of constant size on > the stack would work. > > > > > > > > > > + if (!ipfs_full_data_folder) { > > > > + av_log(h, AV_LOG_DEBUG, "$IPFS_PATH is empty.\n"); > > > > + > > > > + // Try via the home folder. > > > > + home_folder = getenv("HOME"); > > > > + ipfs_full_data_folder = av_asprintf("%s/.ipfs/", > > > > home_folder); > > > > > > Memory leak. This applies to most if not all av_asprintf() calls. > > > > > > > Is there an advised way to neatly clean that up? > > Sure, I can add a bunch of av_free calls to clean it up. But there > > are > > places where it's not as straightforward like where the av_asprintf > > was > > done in an if statement. How do I maintain the knowledge that > > av_asprintf > > was used to call av_free later? > > goto as the others pointed out > > > > > > > > > > > +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)); > > > > > > https://docs.ipfs.io/concepts/ipfs-gateway/ claims ipfs:// is the > > > canonical form. No mentioned is made of any ipfs:{CID} form. > > > Incorrect > > > URLs should be rejected, not silently patched. > > > > > > > I'd like to make a decision here. This current logic (ipfs:// and > > ipfs:, > > same for ipns) is inspired by other protocols that ffmpeg supported. > > I > > simply copied how they work to be consistent. > > Do i: > > 1. keep it as is and be consistent with the rest? > > 2. only allow ipfs:// and ipns://? > > > > Which protocols? This laxness in lavf is worrisome. It breeds laxness > in other projects. > Hmm, I might have been looking at the wrong example and calling that "others do it too". Crypto does it with "crypto:" and "crypto+" From a quick glance that's about it. I'll just remove "ipfs:". I only wanted to stay consistent with what I knew (from crypto) but that is apparently inconsistent. > > /Tomas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@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-requ...@ffmpeg.org with subject "unsubscribe".