Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Soft Works <softworkz@hotmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi
Date: Thu, 9 Jun 2022 19:50:23 +0000
Message-ID: <DM8P223MB0365BAF140B15BE2EF28D468BAA79@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <c5269e6-d682-30d8-9753-748ebae19bf2@martin.st>



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> Storsjö
> Sent: Thursday, June 9, 2022 9:09 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add
> whcartoutf8, wchartoansi and utf8toansi
> 
> On Thu, 9 Jun 2022, Soft Works wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Martin
> >> Storsjö
> >> Sent: Thursday, June 9, 2022 12:10 PM
> >> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h:
> Add
> >> whcartoutf8, wchartoansi and utf8toansi
> >>
> >> On Sun, 5 Jun 2022, Nil Admirari wrote:
> >>
> >>> These functions are going to be used in libavformat/avisynth.c
> >>> and fftools/cmdutils.c to remove MAX_PATH limit.
> >>> ---
> >>> libavutil/wchar_filename.h | 51 ++++++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 51 insertions(+)
> >>
> >> This patchset looks good to me too, thanks! If there's no more comments on
> >> it, I'll push it later.
> >
> > I missed to take another look at it.
> >
> > I'm wondering why it converts wchar back to ansi instead of utf8 in 4/4 and
> > whether it won't fail then, when a path contains non-ANSI chars.
> 
> Yes, that's a preexisting problem there. That patch gets rid of the fixed
> path lengths without touching the rest of the charset handling there.
> 
> There's paths from two sources; getenv() and from GetModuleFileName().
> These are passed to plain fopen() (which uses ANSI filenames).

What I meant is the use of wchartoansi() in patch 4/4.

The current code calls GetModuleFileNameA()
the new code calls GetModuleFileNameW() and then converts the output
file name with wchartoansi().

Can we be sure that this is always the same? 
Especially when the path contains non-ANSI chars?





That's something quite different. GetModuleFileNameA() will return a 
valid and usable path (but it could be an 8.3 path or have some 
replacement chars)

Same applies to GetModuleFileNameW()
when using the wchar output - but not when using a string conversion to
an Ansi string.

> > So the patch in itself is fine - it fixes one aspect, and doesn't make the
> other aspect worse.

Wouldn't it make it worse in the case above?


Thanks,
sw


_______________________________________________
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".

  parent reply	other threads:[~2022-06-09 19:50 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-05 11:35 Nil Admirari
2022-06-05 11:35 ` [FFmpeg-devel] [PATCH v12 2/4] libavformat/avisynth.c: Remove MAX_PATH limit Nil Admirari
2022-06-05 11:35 ` [FFmpeg-devel] [PATCH v12 3/4] compat/w32dlfcn.h: Remove MAX_PATH limit and replace LoadLibraryExA with LoadLibraryExW Nil Admirari
2022-06-05 11:35 ` [FFmpeg-devel] [PATCH v12 4/4] fftools/cmdutils.c: Remove MAX_PATH limit Nil Admirari
2022-06-09 10:10 ` [FFmpeg-devel] [PATCH v12 1/4] libavutil/wchar_filename.h: Add whcartoutf8, wchartoansi and utf8toansi Martin Storsjö
2022-06-09 16:47   ` Soft Works
2022-06-09 16:48     ` Soft Works
2022-06-09 19:09     ` Martin Storsjö
2022-06-09 19:19       ` nil-admirari
2022-06-09 19:50       ` Soft Works [this message]
2022-06-09 20:23         ` Martin Storsjö
2022-06-09 21:12           ` nil-admirari
2022-06-09 21:24             ` Martin Storsjö
2022-06-13 16:36               ` nil-admirari
  -- strict thread matches above, loose matches on Subject: below --
2022-06-05 11:34 Nil Admirari
2022-06-05 11:32 Nil Admirari

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM8P223MB0365BAF140B15BE2EF28D468BAA79@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM \
    --to=softworkz@hotmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git