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

[Mark Gaiser <markg85@gmail.com>]

On Mon, Mar 28, 2022 at 7:00 PM Mark Gaiser <markg85@gmail.com> wrote:

> On Mon, Mar 28, 2022 at 6:44 PM Michael Niedermayer <
> michael@niedermayer.cc> wrote:
>
>> On Mon, Mar 28, 2022 at 06:34:33PM +0200, Mark Gaiser wrote:
>> > On Mon, Mar 28, 2022 at 6:19 PM Michael Niedermayer <
>> michael@niedermayer.cc>
>> > wrote:
>> >
>> > > On Fri, Mar 18, 2022 at 03:50:05PM +0100, Mark Gaiser wrote:
>> > > > This patch adds support for:
>> > > > - ffplay ipfs://<cid>
>> > > > - ffplay ipns://<cid>
>> > > >
>> > > > IPFS data can be played from so called "ipfs gateways".
>> > > > A gateway is essentially a webserver that gives access to the
>> > > > distributed IPFS network.
>> > > >
>> > > > This protocol support (ipfs and ipns) therefore translates
>> > > > ipfs:// and ipns:// to a http:// url. This resulting url is
>> > > > then handled by the http protocol. It could also be https
>> > > > depending on the gateway provided.
>> > > >
>> > > > To use this protocol, a gateway must be provided.
>> > > > If you do nothing it will try to find it in your
>> > > > $HOME/.ipfs/gateway file. The ways to set it manually are:
>> > > > 1. Define a -gateway <url> to the gateway.
>> > > > 2. Define $IPFS_GATEWAY with the full http link to the gateway.
>> > > > 3. Define $IPFS_PATH and point it to the IPFS data path.
>> > > > 4. Have IPFS running in your local user folder (under $HOME/.ipfs).
>> > > >
>> > > > Signed-off-by: Mark Gaiser <markg85@gmail.com>
>> > > > ---
>> > > >  configure                 |   2 +
>> > > >  doc/protocols.texi        |  30 ++++
>> > > >  libavformat/Makefile      |   2 +
>> > > >  libavformat/ipfsgateway.c | 310
>> ++++++++++++++++++++++++++++++++++++++
>> > > >  libavformat/protocols.c   |   2 +
>> > > >  5 files changed, 346 insertions(+)
>> > > >  create mode 100644 libavformat/ipfsgateway.c
>> > >
>> > > Theres some trailing whitespace which needs to be removed
>> > > our git scripts block trailing whitespace in most files
>> > >
>> > > [...]
>> > > > +static int ipfs_close(URLContext *h)
>> > > > +{
>> > > > +    IPFSGatewayContext *c = h->priv_data;
>> > > > +    av_free(c->gateway);
>> > >
>> > > this results in a double free
>> > >
>> >
>> > I believe one of the earlier feedback rounds told me to put it here.
>> > It's not free'd anywhere else.
>>
>> ==22837== Invalid free() / delete / delete[] / realloc()
>> ==22837==    at 0x4C32D3B: free (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==22837==    by 0x117FF88: av_opt_free (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x5C58CF: ffurl_closep (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x5C5AF2: ffurl_close (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x5CA206: avio_close (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x3021C0: ffmpeg_cleanup (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x2F5FA0: exit_program (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x2E34A1: main (in ffmpeg/ffmpeg_g)
>> ==22837==  Address 0x2ced8760 is 0 bytes inside a block of size 17 free'd
>> ==22837==    at 0x4C32D3B: free (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==22837==    by 0x7372BD: ipfs_close (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x5C588C: ffurl_closep (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x5C5AF2: ffurl_close (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x5CA206: avio_close (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x3021C0: ffmpeg_cleanup (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x2F5FA0: exit_program (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x2E34A1: main (in ffmpeg/ffmpeg_g)
>> ==22837==  Block was alloc'd at
>> ==22837==    at 0x4C31A3F: malloc (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==22837==    by 0x4C33D84: realloc (in
>> /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==22837==    by 0x117E120: av_strdup (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x1181DEF: av_opt_set (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x118272D: av_opt_set_dict2 (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x5C598C: ffurl_open_whitelist (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x5CA4DD: ffio_open_whitelist (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x6B88FB: io_open_default (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x5E0E9E: avformat_open_input (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x2EB32B: open_input_file (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x2EF00B: ffmpeg_parse_options (in ffmpeg/ffmpeg_g)
>> ==22837==    by 0x2E3301: main (in ffmpeg/ffmpeg_g)
>>
>>
>> >
>> > Then again, in those earlier rounds I was manipulating c-gateway which
>> > right now isn't the case at all anymore.
>> >
>> > If all that's stopping it from merging is this single line, could you
>> > perhaps merge it and remove this line while at it?
>> > I'm kinda reluctant to make another patch and wait 1-2 weeks again...
>>
>> I do not know why its there or who asked for it to be put there.
>> I dont want to just remove something while merging that someone else
>> asked to be added
>>
>
> It's fine, you can remove it.
> I just checked with crypto.c (which I use as an example).
>
> d->gateway is an AVOption and is never changed in code.
> If I compare it with other AVOption in crypto.c, it too follows the same
> concept of never deleting it manually (probably AVOption magic internally?)
>
> Removing it is fine. Unless crypto.c does it wrong too ;)
> Could you?
> Or do you insist on an updated patch? I would merely remove that one line..
>

A V10 will be here within a few hours.
The diff will be only this single line removed.
I'll retest it locally and rebase it against the current head.

I hope a merge - or even a reply - could be a little more expedited though.
Waiting 1 till 2 weeks is, as one can imagine, not really motivating.


>
>
>>
>> thx
>>
>> [...]
>> --
>> Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>>
>> Never trust a computer, one day, it may think you are the virus. -- Compn
>> _______________________________________________
>> 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".