diff mbox

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

Message ID 6db1f071-2bb1-ece7-f2f6-b64b7342b433@gmail.com
State Accepted
Commit acc9684dcd3949741e944d611f5a2a62db0546e6
Headers show

Commit Message

Gyan Aug. 31, 2018, 4:56 a.m. UTC 
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 <ffmpeg@gyani.pro>
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(-)

Comments

Gyan Sept. 3, 2018, 5:18 a.m. UTC | #1 
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.
Michael Niedermayer Sept. 5, 2018, 12:36 a.m. UTC | #2 
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 ...

[...]
Gyan Sept. 6, 2018, 6:51 p.m. UTC | #3 
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
Gyan Sept. 17, 2018, 8:20 a.m. UTC | #4 
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
diff mbox

Patch

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)