[FFmpeg-devel] [PATCH] avformat/cache - delete cache file after closing handle
Michael Niedermayer
michael at niedermayer.cc
Thu May 23 18:43:07 EEST 2019
On Wed, May 22, 2019 at 09:06:11PM +0530, Gyan wrote:
>
>
> On 22-05-2019 06:37 PM, Nicolas George wrote:
> >Gyan (12019-05-22):
> >>The existing practice of unlinking path immediately after acquiring file
> >>handle was returning (unchecked) EPERM error on Windows. Switched to
> >>deletion after closing handle.
> >>Confirmed that cache protocol temp files are deleted in Windows 7 when
> >>ffmpeg exits.
> >>
> >>The cache protocol calls avutil/avpriv_tempfile(), which in turn, calls
> >>mkstemp (for me), so I don't see the provision to append O_TEMPORARY as a
> >>flag while opening. mkstemp only applies
> >>_O_RDWR | _O_CREAT | _O_EXCL | _O_BINARY
> >>
> >>Should be tested in other environments. If it works, supersedes
> >>https://patchwork.ffmpeg.org/patch/13242/
> >>
> >>Thanks,
> >>Gyan
> >> From f1f17020dd96205a60195e310d8c3b53126c2e00 Mon Sep 17 00:00:00 2001
> >>From: Gyan Doshi <ffmpeg at gyani.pro>
> >>Date: Wed, 22 May 2019 18:05:04 +0530
> >>Subject: [PATCH] avformat/cache - delete cache file after closing handle
> >>
> >>Ensures cache file deletion on Windows, when compiled under MinGW.
> >>---
> >> libavformat/cache.c | 7 ++++++-
> >> 1 file changed, 6 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/libavformat/cache.c b/libavformat/cache.c
> >>index 66bbbf54c9..1db13ceefc 100644
> >>--- a/libavformat/cache.c
> >>+++ b/libavformat/cache.c
> >>@@ -54,6 +54,7 @@ typedef struct CacheEntry {
> >> typedef struct Context {
> >> AVClass *class;
> >> int fd;
> >>+ char *filename;
> >> struct AVTreeNode *root;
> >> int64_t logical_pos;
> >> int64_t cache_pos;
> >>@@ -83,7 +84,7 @@ static int cache_open(URLContext *h, const char *arg, int flags, AVDictionary **
> >> return c->fd;
> >> }
> >>- unlink(buffername);
> >On systems that allow deleting the file when it is still in use deleting
> >it immediately is a better idea: it reduces the risk of leaving it if
> >the application is interrupted prematurely.
> >
> >>+ c->filename = av_strdup(buffername);
> >Missing error check.
> >
> >> av_freep(&buffername);
> >> return ffurl_open_whitelist(&c->inner, arg, flags, &h->interrupt_callback,
> >>@@ -292,11 +293,15 @@ static int enu_free(void *opaque, void *elem)
> >> static int cache_close(URLContext *h)
> >> {
> >> Context *c= h->priv_data;
> >>+ int ret;
> >> av_log(h, AV_LOG_INFO, "Statistics, cache hits:%"PRId64" cache misses:%"PRId64"\n",
> >> c->cache_hit, c->cache_miss);
> >> close(c->fd);
> >>+ ret = avpriv_io_delete(c->filename);
> >>+ if (ret < 0)
> >>+ av_log(h, AV_LOG_ERROR, "Could not delete %s. Error is %s\n", c->filename, av_err2str(ret));
> >> ffurl_close(c->inner);
> >> av_tree_enumerate(c->root, NULL, NULL, enu_free);
> >> av_tree_destroy(c->root);
>
> Revised patch.
>
> Gyan
>
>
> cache.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
> 3cb3458fccd0c253c7ab0d281935145beb03ab25 v2-0001-avformat-cache-delete-cache-file-after-closing-ha.patch
> From 54b24428fcf6b513574f5c1b626ebb505b123d77 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <ffmpeg at gyani.pro>
> Date: Wed, 22 May 2019 18:05:04 +0530
> Subject: [PATCH v2] avformat/cache - delete cache file after closing handle
>
> Ensures cache file deletion on Windows, when compiled under MinGW.
> ---
> libavformat/cache.c | 21 ++++++++++++++++++++-
> 1 file changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/cache.c b/libavformat/cache.c
> index 66bbbf54c9..ee1b868157 100644
> --- a/libavformat/cache.c
> +++ b/libavformat/cache.c
> @@ -54,6 +54,7 @@ typedef struct CacheEntry {
> typedef struct Context {
> AVClass *class;
> int fd;
> + char *filename;
> struct AVTreeNode *root;
> int64_t logical_pos;
> int64_t cache_pos;
> @@ -72,6 +73,7 @@ static int cmp(const void *key, const void *node)
>
> static int cache_open(URLContext *h, const char *arg, int flags, AVDictionary **options)
> {
> + int ret;
> char *buffername;
> Context *c= h->priv_data;
>
> @@ -83,7 +85,18 @@ static int cache_open(URLContext *h, const char *arg, int flags, AVDictionary **
> return c->fd;
> }
>
> - unlink(buffername);
> + ret = avpriv_io_delete(buffername);
> +
> + if (ret < 0) {
> + c->filename = av_strdup(buffername);
> + if (!c->filename) {
> + close(c->fd);
> + avpriv_io_delete(buffername);
> + av_freep(&buffername);
> + return AVERROR(ENOMEM);
> + }
> + }
you can use buffername directly and skip freeing it instead of strdup()
this avoids the need for the error handling
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
No great genius has ever existed without some touch of madness. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190523/76316d07/attachment.sig>
More information about the ffmpeg-devel
mailing list