| 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 |
| 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 |
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
> -----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
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
> -----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 --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];