[FFmpeg-devel] [PATCH] Make the ffmpeg "Overwrite ? [y/N]" question work with more than one file to overwrite

[Stefano Sabatini]

On date Wednesday 2009-03-18 06:26:52 +0100, Michael Niedermayer encoded:
> On Sat, Mar 14, 2009 at 10:59:56AM +0100, Stefano Sabatini wrote:
> > On date Friday 2009-03-13 18:32:40 -0700, Baptiste Coudurier encoded:
> > > On 3/13/2009 6:30 PM, Michael Niedermayer wrote:
> > > > On Sat, Mar 14, 2009 at 01:14:13AM +0100, Stefano Sabatini wrote:
> > > >> On date Wednesday 2009-03-11 01:42:10 +0100, Michael Niedermayer encoded:
> > > >>> On Tue, Mar 10, 2009 at 09:36:00PM +0100, Stefano Sabatini wrote:
> > > >>>> Hi all,
> > > >>>>
> > > >>>> I recently stepped into this:
> > > >>>> stefano at geppetto ~/s/ffmpeg> ffmpeg -i in123.mpeg out0.avi out1.ogg -y -map 0:2 -map 0:1
> > > >>>> [...]
> > > >>>> Input #0, mpeg, from 'in123.mpeg':
> > > >>>>   Duration: 00:03:40.05, start: 0.500000, bitrate: 81 kb/s
> > > >>>>     Stream #0.0[0x1c0]: Audio: mp2, 48000 Hz, stereo, s16, 64 kb/s
> > > >>>>     Stream #0.1[0x1c1]: Audio: mp2, 48000 Hz, stereo, s16, 64 kb/s
> > > >>>>     Stream #0.2[0x1c2]: Audio: mp2, 48000 Hz, stereo, s16, 64 kb/s
> > > >>>> File 'out0.avi' already exists. Overwrite ? [y/N] y   
> > > >>>> File 'out1.ogg' already exists. Overwrite ? [y/N]
> > > >>>> Not overwriting - exiting
> > > >>>>
> > > >>>> After the first question is replied, the stdin buffer still contains
> > > >>>> the '\n' buffered, then ffmpeg read from it with getchar(), get a
> > > >>>> '\n', which is different from 'Y' and abort immediately.
> > > >>>>
> > > >>>> Patch fixes it.
> > > >>>>
> > > >>>> Regards.
> > > >>>> -- 
> > > >>>> FFmpeg = Fast and Faboulous Most Picky Energized Gorilla
> > > >>>> Index: ffmpeg.c
> > > >>>> ===================================================================
> > > >>>> --- ffmpeg.c	(revision 17926)
> > > >>>> +++ ffmpeg.c	(working copy)
> > > >>>> @@ -3362,12 +3362,13 @@
> > > >>>>               filename[1] == ':' ||
> > > >>>>               av_strstart(filename, "file:", NULL))) {
> > > >>>>              if (url_exist(filename)) {
> > > >>>> -                int c;
> > > >>>> +                char c, line[256];
> > > >>>>  
> > > >>>>                  if (!using_stdin) {
> > > >>>>                      fprintf(stderr,"File '%s' already exists. Overwrite ? [y/N] ", filename);
> > > >>>>                      fflush(stderr);
> > > >>>> -                    c = getchar();
> > > >>>> +                    fgets(line, sizeof(line), stdin);
> > > >>>> +                    sscanf(line, "%c", &c);
> > > >>> what weird mess is this ?
> > > >>> getchar();getchar()
> > > >> underkill, what if the user prints more than one character?
> > > > 
> > > > if the user enters more than 2 chars to a question asking for one
> > > > then interpreting this as 'y' is maybe not such a good idea.
> > 
> > This is the behaviour of many programs which reads yes/no repyies from
> > stdin, in particular I refer to the coreutils.
> > 
> > But we can easily change this, and force the user to provide any
> > case-insensitive substring of "yes" for example.
> > 
> > > > also, from a user POV, just having to press y and no enter would be nice
> > 
> > That's potentially dangerous, IMO is safer to let the user *explicitely*
> > press enter to confirm.
> > 
> > > > 
> > > 
> > > Weee termcap :>
> > > Don't forget mingw though.
> > 
> > Anyway if we decide that this is a good idea, we can extend read_yes()
> > to support that.
> > 
> > Regards.
> > -- 
> > FFmpeg = Frightening & Fiendish Mastodontic Pitiful Enigmatic Gymnast
> 
> > Index: cmdutils.h
> > ===================================================================
> > --- cmdutils.h	(revision 17951)
> > +++ cmdutils.h	(working copy)
> > @@ -152,4 +152,10 @@
> >   */
> >  void show_formats(void);
> >  
> > +/**
> > + * Returns a positive value if reads a line starting with [yY] from
> > + * standard input, 0 otherwise.
> > + */
> > +int read_yes(void);
> 
> read_yesno()
> 
> also factorization of code and bugfix should be seperate
> seperation as
> 1. duplicate fixed code
> 2. replace broken code be duplicated code
> is not ok

Check again.

Regards.
-- 
FFmpeg = Faboulous Foolish Minimal Purposeless Extreme Gadget
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-read-answ.patch
Type: text/x-diff
Size: 584 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090318/82481959/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: implement-read-yesno.patch
Type: text/x-diff
Size: 956 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090318/82481959/attachment-0001.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-use-read-yesno.patch
Type: text/x-diff
Size: 1036 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090318/82481959/attachment-0002.patch>