Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On 07-09-2018 12:21 AM, Gyan Doshi wrote: On 05-09-2018 06:06 AM, Michael Niedermayer wrote: On Mon, Sep 03, 2018 at 10:48:45AM +0530, Gyan Doshi wrote: On 31-08-2018 10:26 AM, Gyan Doshi wrote: On 31-08-2018 09:57 AM, Gyan Doshi wrote: On 31-08-2018 04:28 AM, Marton Balint wrote: Is there any real use case when same source and destination works, so the option can be used? If not, then just make ffmpeg fail, like the cp command fails for same source and destination. I am against adding an option if it has no known use. Via the file protocol, not that I know of. Will remove. Gyan Revised patch attached. Ping. no objections though this solution has limitations, so if someone has a better idea ... Looks like no one does. Plan to push tomorrow. Pushed as acc9684dcd3949741e944d611f5a2a62db0546e6 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On 05-09-2018 06:06 AM, Michael Niedermayer wrote: On Mon, Sep 03, 2018 at 10:48:45AM +0530, Gyan Doshi wrote: On 31-08-2018 10:26 AM, Gyan Doshi wrote: On 31-08-2018 09:57 AM, Gyan Doshi wrote: On 31-08-2018 04:28 AM, Marton Balint wrote: Is there any real use case when same source and destination works, so the option can be used? If not, then just make ffmpeg fail, like the cp command fails for same source and destination. I am against adding an option if it has no known use. Via the file protocol, not that I know of. Will remove. Gyan Revised patch attached. Ping. no objections though this solution has limitations, so if someone has a better idea ... Looks like no one does. Plan to push tomorrow. Thanks, Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On Mon, Sep 03, 2018 at 10:48:45AM +0530, Gyan Doshi wrote: > On 31-08-2018 10:26 AM, Gyan Doshi wrote: > >On 31-08-2018 09:57 AM, Gyan Doshi wrote: > >>On 31-08-2018 04:28 AM, Marton Balint wrote: > >> > >>> > >>>Is there any real use case when same source and destination works, so > >>>the option can be used? > >>> > >>>If not, then just make ffmpeg fail, like the cp command fails for same > >>>source and destination. I am against adding an option if it has no > >>>known use. > >> > >>Via the file protocol, not that I know of. Will remove. > >> > >>Gyan > > > >Revised patch attached. > > Ping. no objections though this solution has limitations, so if someone has a better idea ... [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On 31-08-2018 10:26 AM, Gyan Doshi wrote: On 31-08-2018 09:57 AM, Gyan Doshi wrote: On 31-08-2018 04:28 AM, Marton Balint wrote: Is there any real use case when same source and destination works, so the option can be used? If not, then just make ffmpeg fail, like the cp command fails for same source and destination. I am against adding an option if it has no known use. Via the file protocol, not that I know of. Will remove. Gyan Revised patch attached. Ping. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On 31-08-2018 09:57 AM, Gyan Doshi wrote: On 31-08-2018 04:28 AM, Marton Balint wrote: Is there any real use case when same source and destination works, so the option can be used? If not, then just make ffmpeg fail, like the cp command fails for same source and destination. I am against adding an option if it has no known use. Via the file protocol, not that I know of. Will remove. Gyan Revised patch attached. From 1422de3eecb921201727e90306edbe1ff6f26947 Mon Sep 17 00:00:00 2001 From: Gyan Doshi Date: Sun, 26 Aug 2018 11:22:50 +0530 Subject: [PATCH v2] ffmpeg: block output == input for files Fixes #4655 --- fftools/ffmpeg_opt.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index 58ec13e5a8..c44ed63730 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -900,13 +900,14 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic) static void assert_file_overwrite(const char *filename) { +const char *proto_name = avio_find_protocol_name(filename); + if (file_overwrite && no_file_overwrite) { fprintf(stderr, "Error, both -y and -n supplied. Exiting.\n"); exit_program(1); } if (!file_overwrite) { -const char *proto_name = avio_find_protocol_name(filename); if (proto_name && !strcmp(proto_name, "file") && avio_check(filename, 0) == 0) { if (stdin_interaction && !no_file_overwrite) { fprintf(stderr,"File '%s' already exists. Overwrite ? [y/N] ", filename); @@ -925,6 +926,19 @@ static void assert_file_overwrite(const char *filename) } } } + +if (proto_name && !strcmp(proto_name, "file")) { +for (int i = 0; i < nb_input_files; i++) { + InputFile *file = input_files[i]; + if (file->ctx->iformat->flags & AVFMT_NOFILE) + continue; + if (!strcmp(filename, file->ctx->url)) { + av_log(NULL, AV_LOG_FATAL, "Output %s same as Input #%d - exiting\n", filename, i); + av_log(NULL, AV_LOG_WARNING, "FFmpeg cannot edit existing files in-place.\n"); + exit_program(1); + } +} +} } static void dump_attachment(AVStream *st, const char *filename) -- 2.18.0___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On 31-08-2018 04:28 AM, Marton Balint wrote: Is there any real use case when same source and destination works, so the option can be used? If not, then just make ffmpeg fail, like the cp command fails for same source and destination. I am against adding an option if it has no known use. Via the file protocol, not that I know of. Will remove. Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On Thu, 30 Aug 2018, Paul B Mahol wrote: On 8/30/18, Gyan Doshi wrote: On 29-08-2018 09:48 AM, Gyan Doshi wrote: On 29-08-2018 02:43 AM, Michael Niedermayer wrote: On Tue, Aug 28, 2018 at 08:31:51AM +0200, Marton Balint wrote: Instead of this, maybe we should add support to write lock the files when opening them for reading. Then ffmpeg can request this. That would be an useful option, and not just for unexperienced users. depending on how this is done, it could interfere with reading a file while it is being written/appeneded to by another process Yes, a user may use a growing MPEG-TS file as input. Also, is there a universal & portable way to secure a write lock across all platforms? I think a 'soft' solution is preferable. Any suggestions or objections? If no, will push in a day. Please do not push, until consensus is reached. What about red log warning text? Is there any real use case when same source and destination works, so the option can be used? If not, then just make ffmpeg fail, like the cp command fails for same source and destination. I am against adding an option if it has no known use. Thanks, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On 8/30/18, Gyan Doshi wrote: > On 29-08-2018 09:48 AM, Gyan Doshi wrote: >> On 29-08-2018 02:43 AM, Michael Niedermayer wrote: >>> On Tue, Aug 28, 2018 at 08:31:51AM +0200, Marton Balint wrote: >> Instead of this, maybe we should add support to write lock the files when opening them for reading. Then ffmpeg can request this. That would be an useful option, and not just for unexperienced users. >>> >>> depending on how this is done, it could interfere with reading a file >>> while >>> it is being written/appeneded to by another process >> >> Yes, a user may use a growing MPEG-TS file as input. >> >> Also, is there a universal & portable way to secure a write lock across >> all platforms? >> >> I think a 'soft' solution is preferable. > > Any suggestions or objections? > > If no, will push in a day. Please do not push, until consensus is reached. What about red log warning text? ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On 29-08-2018 09:48 AM, Gyan Doshi wrote: On 29-08-2018 02:43 AM, Michael Niedermayer wrote: On Tue, Aug 28, 2018 at 08:31:51AM +0200, Marton Balint wrote: Instead of this, maybe we should add support to write lock the files when opening them for reading. Then ffmpeg can request this. That would be an useful option, and not just for unexperienced users. depending on how this is done, it could interfere with reading a file while it is being written/appeneded to by another process Yes, a user may use a growing MPEG-TS file as input. Also, is there a universal & portable way to secure a write lock across all platforms? I think a 'soft' solution is preferable. Any suggestions or objections? If no, will push in a day. Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On 29-08-2018 02:43 AM, Michael Niedermayer wrote: On Tue, Aug 28, 2018 at 08:31:51AM +0200, Marton Balint wrote: Instead of this, maybe we should add support to write lock the files when opening them for reading. Then ffmpeg can request this. That would be an useful option, and not just for unexperienced users. depending on how this is done, it could interfere with reading a file while it is being written/appeneded to by another process Yes, a user may use a growing MPEG-TS file as input. Also, is there a universal & portable way to secure a write lock across all platforms? I think a 'soft' solution is preferable. Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On Tue, Aug 28, 2018 at 08:31:51AM +0200, Marton Balint wrote: > > > On Tue, 28 Aug 2018, Gyan Doshi wrote: > > > > >With some regularity, we have users trying to update input files using > >ffmpeg, usually for the purposes of tagging, but occasionally for changing > >the encoding or something else. Other apps like mp4box or taggers edit the > >files in-place i.e. destination file is same as the source. FFmpeg cannot > >do this. But since these users don't realize that, they will answer Yes to > >the overwrite prompt and then discover their source has been destroyed. > > > >Attached patch checks the URL for file protocol outputs against inputs and > >aborts upon a match. An option is provided for the user to force the > >operation. > > > >The check isn't robust. In particular, it looks for exact url string > >matches, so a command like > > > > ffmpeg -i file:somefile -some_op somefile > > > >will still pass through. But I consider such invocations rare. Most times > >I've seen users trying this (on Stackexchange or other support forums), > >the command is typically of the form, > > > > for i; do ffmpeg -i $i -some_op -y $i > > > >Such a scenario was filed as a bug in #4655 but it was marked as wontfix > >since some protocols/services can manage bidir ops to the same endpoint. > >For that reason, everything other than file protocol sources/sinks are > >exempt i.e. http, pipes, devices..etc. > > > >This patch doesn't affect the semantics of '-y' and adds the check after it. > > Instead of this, maybe we should add support to write lock the files when > opening them for reading. Then ffmpeg can request this. That would be an > useful option, and not just for unexperienced users. depending on how this is done, it could interfere with reading a file while it is being written/appeneded to by another process [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Good people do not need laws to tell them to act responsibly, while bad people will find a way around the laws. -- Plato signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
On Tue, 28 Aug 2018, Gyan Doshi wrote: With some regularity, we have users trying to update input files using ffmpeg, usually for the purposes of tagging, but occasionally for changing the encoding or something else. Other apps like mp4box or taggers edit the files in-place i.e. destination file is same as the source. FFmpeg cannot do this. But since these users don't realize that, they will answer Yes to the overwrite prompt and then discover their source has been destroyed. Attached patch checks the URL for file protocol outputs against inputs and aborts upon a match. An option is provided for the user to force the operation. The check isn't robust. In particular, it looks for exact url string matches, so a command like ffmpeg -i file:somefile -some_op somefile will still pass through. But I consider such invocations rare. Most times I've seen users trying this (on Stackexchange or other support forums), the command is typically of the form, for i; do ffmpeg -i $i -some_op -y $i Such a scenario was filed as a bug in #4655 but it was marked as wontfix since some protocols/services can manage bidir ops to the same endpoint. For that reason, everything other than file protocol sources/sinks are exempt i.e. http, pipes, devices..etc. This patch doesn't affect the semantics of '-y' and adds the check after it. Instead of this, maybe we should add support to write lock the files when opening them for reading. Then ffmpeg can request this. That would be an useful option, and not just for unexperienced users. Regards, Marton ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH] ffmpeg: block output == input for files
With some regularity, we have users trying to update input files using ffmpeg, usually for the purposes of tagging, but occasionally for changing the encoding or something else. Other apps like mp4box or taggers edit the files in-place i.e. destination file is same as the source. FFmpeg cannot do this. But since these users don't realize that, they will answer Yes to the overwrite prompt and then discover their source has been destroyed. Attached patch checks the URL for file protocol outputs against inputs and aborts upon a match. An option is provided for the user to force the operation. The check isn't robust. In particular, it looks for exact url string matches, so a command like ffmpeg -i file:somefile -some_op somefile will still pass through. But I consider such invocations rare. Most times I've seen users trying this (on Stackexchange or other support forums), the command is typically of the form, for i; do ffmpeg -i $i -some_op -y $i Such a scenario was filed as a bug in #4655 but it was marked as wontfix since some protocols/services can manage bidir ops to the same endpoint. For that reason, everything other than file protocol sources/sinks are exempt i.e. http, pipes, devices..etc. This patch doesn't affect the semantics of '-y' and adds the check after it. Thanks, Gyan From deb97d9b5dd1f50a7e6b560c77b81b9cd707b7c1 Mon Sep 17 00:00:00 2001 From: Gyan Doshi Date: Sun, 26 Aug 2018 11:22:50 +0530 Subject: [PATCH] ffmpeg: block output == input for files Add option write_to_input_file to allow it. Fixes #4655 --- doc/ffmpeg.texi | 6 ++ fftools/ffmpeg.h | 1 + fftools/ffmpeg_opt.c | 20 +++- 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/doc/ffmpeg.texi b/doc/ffmpeg.texi index 3717f22d42..6fb359eabe 100644 --- a/doc/ffmpeg.texi +++ b/doc/ffmpeg.texi @@ -445,6 +445,12 @@ Overwrite output files without asking. Do not overwrite output files, and exit immediately if a specified output file already exists. +@item -write_to_input_file (@emph{global}) +Allow writing to an input file. Normally, this will destroy the input file and +the conversion will fail, as FFmpeg does not perform in-place editing. But some +protocols or storage services can accommodate this use-case. Verify before setting it. +This option does not override the generic output overwrite check. Disabled by default. + @item -stream_loop @var{number} (@emph{input}) Set number of times input stream shall be looped. Loop 0 means no loop, loop -1 means infinite loop. diff --git a/fftools/ffmpeg.h b/fftools/ffmpeg.h index eb1eaf6363..6c1f87c7ad 100644 --- a/fftools/ffmpeg.h +++ b/fftools/ffmpeg.h @@ -606,6 +606,7 @@ extern int frame_bits_per_raw_sample; extern AVIOContext *progress_avio; extern float max_error_rate; extern char *videotoolbox_pixfmt; +extern int write_to_input_file; extern int filter_nbthreads; extern int filter_complex_nbthreads; diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c index 58ec13e5a8..74e9b94bf5 100644 --- a/fftools/ffmpeg_opt.c +++ b/fftools/ffmpeg_opt.c @@ -121,6 +121,7 @@ static int input_stream_potentially_available = 0; static int ignore_unknown_streams = 0; static int copy_unknown_streams = 0; static int find_stream_info = 1; +extern int write_to_input_file = 0; static void uninit_options(OptionsContext *o) { @@ -900,13 +901,14 @@ static void add_input_streams(OptionsContext *o, AVFormatContext *ic) static void assert_file_overwrite(const char *filename) { +const char *proto_name = avio_find_protocol_name(filename); + if (file_overwrite && no_file_overwrite) { fprintf(stderr, "Error, both -y and -n supplied. Exiting.\n"); exit_program(1); } if (!file_overwrite) { -const char *proto_name = avio_find_protocol_name(filename); if (proto_name && !strcmp(proto_name, "file") && avio_check(filename, 0) == 0) { if (stdin_interaction && !no_file_overwrite) { fprintf(stderr,"File '%s' already exists. Overwrite ? [y/N] ", filename); @@ -925,6 +927,20 @@ static void assert_file_overwrite(const char *filename) } } } + +if (!write_to_input_file && proto_name && !strcmp(proto_name, "file")) { +for (int i = 0; i < nb_input_files; i++) { + InputFile *file = input_files[i]; + if (file->ctx->iformat->flags & AVFMT_NOFILE) + continue; + if (!strcmp(filename, file->ctx->url)) { + av_log(NULL, AV_LOG_FATAL, "Output same as Input #%d - exiting\n", i); + av_log(NULL, AV_LOG_WARNING, "FFmpeg cannot edit existing files in-place.\n" +"Add -write_to_input_file if this is a safe operation and intended.\n"); + exit_program(1); + } +} +} } static void dump_attachment(AVStream *st, const char *filename) @@ -3315,6 +3331,8 @@ const OptionDef opti