diff mbox series

[FFmpeg-devel,v2] avfilter: use av_fopen_utf8() instead of plain fopen()

Message ID pull.26.v2.ffstaging.FFmpeg.1652122322889.ffmpegagent@gmail.com
State New
Headers show
Series [FFmpeg-devel,v2] avfilter: use av_fopen_utf8() instead of plain fopen() | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

softworkz May 9, 2022, 6:52 p.m. UTC
From: softworkz <softworkz@hotmail.com>

Signed-off-by: softworkz <softworkz@hotmail.com>
---
    use av_fopen_utf8() instead of plain fopen()
    
    Unify file access operations by replacing usages of direct calls to
    posix fopen()
    
    v2: Remove changes to fftools for now

Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-26%2Fsoftworkz%2Fsubmit_replace_fopen-v2
Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-26/softworkz/submit_replace_fopen-v2
Pull-Request: https://github.com/ffstaging/FFmpeg/pull/26

Range-diff vs v1:

 1:  5802c8526c < -:  ---------- fftools: use av_fopen_utf8() instead of plain fopen()
 2:  3266640a93 = 1:  e47287be64 avfilter: use av_fopen_utf8() instead of plain fopen()


 libavfilter/af_firequalizer.c | 2 +-
 libavfilter/vf_deshake.c      | 2 +-
 libavfilter/vf_signature.c    | 4 ++--
 libavfilter/vf_ssim.c         | 2 +-
 libavfilter/vf_vmafmotion.c   | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)


base-commit: f3b7ba21ba49b32b4476a8c7c5a9bcdad15e3943

Comments

Martin Storsjö May 10, 2022, 8:12 p.m. UTC | #1
On Mon, 9 May 2022, softworkz wrote:

> From: softworkz <softworkz@hotmail.com>
>
> Signed-off-by: softworkz <softworkz@hotmail.com>
> ---
>    use av_fopen_utf8() instead of plain fopen()
>
>    Unify file access operations by replacing usages of direct calls to
>    posix fopen()
>
>    v2: Remove changes to fftools for now
>
> Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-ffstaging-26%2Fsoftworkz%2Fsubmit_replace_fopen-v2
> Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-ffstaging-26/softworkz/submit_replace_fopen-v2
> Pull-Request: https://github.com/ffstaging/FFmpeg/pull/26
>
> Range-diff vs v1:
>
> 1:  5802c8526c < -:  ---------- fftools: use av_fopen_utf8() instead of plain fopen()
> 2:  3266640a93 = 1:  e47287be64 avfilter: use av_fopen_utf8() instead of plain fopen()
>
>
> libavfilter/af_firequalizer.c | 2 +-
> libavfilter/vf_deshake.c      | 2 +-
> libavfilter/vf_signature.c    | 4 ++--
> libavfilter/vf_ssim.c         | 2 +-
> libavfilter/vf_vmafmotion.c   | 2 +-
> 5 files changed, 6 insertions(+), 6 deletions(-)

LGTM I think. For fully fixing the situation about this function, I 
believe we're going to need to rename it (as the proper solution won't be 
a public function), but as there's already some uses, it's probably fine 
to first take it into use consistently, and then rename all the 
occurrances later.

But we should probably add a copy of file_open.o in libavfilter too (as 
you noted). This is indeed a preexisting problem, but the issue will 
become more visible if we use it in more places.

// Martin
Soft Works May 10, 2022, 11:03 p.m. UTC | #2
> -----Original Message-----
> From: Martin Storsjö <martin@martin.st>
> Sent: Tuesday, May 10, 2022 10:12 PM
> To: softworkz <ffmpegagent@gmail.com>
> Cc: ffmpeg-devel@ffmpeg.org; Soft Works <softworkz@hotmail.com>
> Subject: Re: [PATCH v2] avfilter: use av_fopen_utf8() instead of plain
> fopen()
> 
> On Mon, 9 May 2022, softworkz wrote:
> 
> > From: softworkz <softworkz@hotmail.com>
> >
> > Signed-off-by: softworkz <softworkz@hotmail.com>
> > ---
> >    use av_fopen_utf8() instead of plain fopen()
> >
> >    Unify file access operations by replacing usages of direct calls
> to
> >    posix fopen()
> >
> >    v2: Remove changes to fftools for now
> >
> > Published-As: https://github.com/ffstaging/FFmpeg/releases/tag/pr-
> ffstaging-26%2Fsoftworkz%2Fsubmit_replace_fopen-v2
> > Fetch-It-Via: git fetch https://github.com/ffstaging/FFmpeg pr-
> ffstaging-26/softworkz/submit_replace_fopen-v2
> > Pull-Request: https://github.com/ffstaging/FFmpeg/pull/26
> >
> > Range-diff vs v1:
> >
> > 1:  5802c8526c < -:  ---------- fftools: use av_fopen_utf8() instead
> of plain fopen()
> > 2:  3266640a93 = 1:  e47287be64 avfilter: use av_fopen_utf8()
> instead of plain fopen()
> >
> >
> > libavfilter/af_firequalizer.c | 2 +-
> > libavfilter/vf_deshake.c      | 2 +-
> > libavfilter/vf_signature.c    | 4 ++--
> > libavfilter/vf_ssim.c         | 2 +-
> > libavfilter/vf_vmafmotion.c   | 2 +-
> > 5 files changed, 6 insertions(+), 6 deletions(-)
> 
> LGTM I think. For fully fixing the situation about this function, I
> believe we're going to need to rename it (as the proper solution won't
> be
> a public function), but as there's already some uses, it's probably
> fine
> to first take it into use consistently, and then rename all the
> occurrances later.

Makes sense. Thanks for reviewing.

> But we should probably add a copy of file_open.o in libavfilter too
> (as
> you noted). This is indeed a preexisting problem, but the issue will
> become more visible if we use it in more places.

Would you able to submit a patch for this or shall I?
(I'd prefer the former ;-)

Thanks,
softworkz
Tobias Rapp May 17, 2022, 11:56 a.m. UTC | #3
On 17/05/2022 02:39, ffmpegagent wrote:
> Unify file access operations by replacing usages of direct calls to posix
> fopen()

As the cover letter will be gone after commit I think it would be a good 
idea to add the reason for the change also in the commit messages directly.

Is this in preparation for the long filename support on Windows? Then 
maybe this could be mentioned, too.

> v2: Remove changes to fftools for now
> v3: Add some additional replacements
> 
> softworkz (2):
>    avfilter: use av_fopen_utf8() instead of plain fopen()
>    avfilter/dvdsubdec: use av_fopen_utf8() instead of plain fopen()

Patch #2 title should start with "avcodec/dvdsubdec: ..." I guess.

> 
>   libavcodec/dvdsubdec.c            | 2 +-
>   libavfilter/af_firequalizer.c     | 2 +-
>   libavfilter/vf_deshake.c          | 2 +-
>   libavfilter/vf_psnr.c             | 2 +-
>   libavfilter/vf_signature.c        | 4 ++--
>   libavfilter/vf_ssim.c             | 2 +-
>   libavfilter/vf_vidstabdetect.c    | 2 +-
>   libavfilter/vf_vidstabtransform.c | 2 +-
>   libavfilter/vf_vmafmotion.c       | 2 +-
>   9 files changed, 10 insertions(+), 10 deletions(-)
> 
> [...]

Regards,
Tobias
Soft Works May 17, 2022, 12:26 p.m. UTC | #4
> -----Original Message-----
> From: Tobias Rapp <t.rapp@noa-archive.com>
> Sent: Tuesday, May 17, 2022 1:56 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>; ffmpegagent <ffmpegagent@gmail.com>
> Cc: Martin Storsjö <martin@martin.st>; softworkz
> <softworkz@hotmail.com>
> Subject: Re: [FFmpeg-devel] [PATCH v3 0/2] use av_fopen_utf8() instead
> of plain fopen()
> 
> On 17/05/2022 02:39, ffmpegagent wrote:
> > Unify file access operations by replacing usages of direct calls to
> posix
> > fopen()
> 
> As the cover letter will be gone after commit I think it would be a
> good
> idea to add the reason for the change also in the commit messages
> directly.
> 
> Is this in preparation for the long filename support on Windows? Then
> maybe this could be mentioned, too.
> 
> > v2: Remove changes to fftools for now
> > v3: Add some additional replacements
> >
> > softworkz (2):
> >    avfilter: use av_fopen_utf8() instead of plain fopen()
> >    avfilter/dvdsubdec: use av_fopen_utf8() instead of plain fopen()
> 
> Patch #2 title should start with "avcodec/dvdsubdec: ..." I guess.

Yes, yes and yes. ;-)

Thanks a lot for reviewing. Update will follow.

Kind regards,
softworkz
diff mbox series

Patch

diff --git a/libavfilter/af_firequalizer.c b/libavfilter/af_firequalizer.c
index c19a2fe122..e1497dcef0 100644
--- a/libavfilter/af_firequalizer.c
+++ b/libavfilter/af_firequalizer.c
@@ -604,7 +604,7 @@  static int generate_kernel(AVFilterContext *ctx, const char *gain, const char *g
     if (ret < 0)
         return ret;
 
-    if (s->dumpfile && (!s->dump_buf || !s->analysis_rdft || !(dump_fp = fopen(s->dumpfile, "w"))))
+    if (s->dumpfile && (!s->dump_buf || !s->analysis_rdft || !(dump_fp = av_fopen_utf8(s->dumpfile, "w"))))
         av_log(ctx, AV_LOG_WARNING, "dumping failed.\n");
 
     vars[VAR_CHS] = inlink->ch_layout.nb_channels;
diff --git a/libavfilter/vf_deshake.c b/libavfilter/vf_deshake.c
index 4f28467bb2..dea69e11cd 100644
--- a/libavfilter/vf_deshake.c
+++ b/libavfilter/vf_deshake.c
@@ -353,7 +353,7 @@  static av_cold int init(AVFilterContext *ctx)
     }
 
     if (deshake->filename)
-        deshake->fp = fopen(deshake->filename, "w");
+        deshake->fp = av_fopen_utf8(deshake->filename, "w");
     if (deshake->fp)
         fwrite("Ori x, Avg x, Fin x, Ori y, Avg y, Fin y, Ori angle, Avg angle, Fin angle, Ori zoom, Avg zoom, Fin zoom\n", 1, 104, deshake->fp);
 
diff --git a/libavfilter/vf_signature.c b/libavfilter/vf_signature.c
index 4ca57ebf1d..904123fd4e 100644
--- a/libavfilter/vf_signature.c
+++ b/libavfilter/vf_signature.c
@@ -386,7 +386,7 @@  static int xml_export(AVFilterContext *ctx, StreamContext *sc, const char* filen
     FILE* f;
     unsigned int pot3[5] = { 3*3*3*3, 3*3*3, 3*3, 3, 1 };
 
-    f = fopen(filename, "w");
+    f = av_fopen_utf8(filename, "w");
     if (!f) {
         int err = AVERROR(EINVAL);
         char buf[128];
@@ -500,7 +500,7 @@  static int binary_export(AVFilterContext *ctx, StreamContext *sc, const char* fi
     if (!buffer)
         return AVERROR(ENOMEM);
 
-    f = fopen(filename, "wb");
+    f = av_fopen_utf8(filename, "wb");
     if (!f) {
         int err = AVERROR(EINVAL);
         char buf[128];
diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c
index 32f313817d..4a82cbec06 100644
--- a/libavfilter/vf_ssim.c
+++ b/libavfilter/vf_ssim.c
@@ -404,7 +404,7 @@  static av_cold int init(AVFilterContext *ctx)
         if (!strcmp(s->stats_file_str, "-")) {
             s->stats_file = stdout;
         } else {
-            s->stats_file = fopen(s->stats_file_str, "w");
+            s->stats_file = av_fopen_utf8(s->stats_file_str, "w");
             if (!s->stats_file) {
                 int err = AVERROR(errno);
                 char buf[128];
diff --git a/libavfilter/vf_vmafmotion.c b/libavfilter/vf_vmafmotion.c
index 8b7e9b17ef..2ac67e5935 100644
--- a/libavfilter/vf_vmafmotion.c
+++ b/libavfilter/vf_vmafmotion.c
@@ -312,7 +312,7 @@  static av_cold int init(AVFilterContext *ctx)
         if (!strcmp(s->stats_file_str, "-")) {
             s->stats_file = stdout;
         } else {
-            s->stats_file = fopen(s->stats_file_str, "w");
+            s->stats_file = av_fopen_utf8(s->stats_file_str, "w");
             if (!s->stats_file) {
                 int err = AVERROR(errno);
                 char buf[128];