Re: [FFmpeg-devel] [PATCH] ffmpeg: block output == input for files

2018-09-17 Thread Gyan Doshi

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

2018-09-06 Thread Gyan Doshi

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

2018-09-04 Thread Michael Niedermayer
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

2018-09-02 Thread Gyan Doshi

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

2018-08-30 Thread Gyan Doshi

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

2018-08-30 Thread Gyan Doshi

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

2018-08-30 Thread Marton Balint



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

2018-08-30 Thread Paul B Mahol
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

2018-08-30 Thread Gyan Doshi

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

2018-08-28 Thread Gyan Doshi

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

2018-08-28 Thread Michael Niedermayer
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

2018-08-28 Thread Marton Balint



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

2018-08-27 Thread Gyan Doshi


 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