[FFmpeg-devel] [RFC][PATCH 2/2] ffmpeg_opt: assert_file_overwrite: Work for all file protocol outputs

[Stefano Sabatini]

On date Monday 2014-01-06 17:15:18 +0100, Alexander Strasser encoded:
> On 2014-01-06 00:11 +0100, Stefano Sabatini wrote:
> > On date Sunday 2014-01-05 03:30:27 +0100, Alexander Strasser encoded:
> > > Currently the file overwrite check does not work for paths that
> > > contain a colon.
> > > 
> > > Use avio_find_protocol to always execute the existence check if
> > > the file protocol is going to be used.
> > > 
> > > Fix other part of ticket #3249.
> > > 
> > > Signed-off-by: Alexander Strasser <eclipse7 at gmx.net>
> > > ---
> > >  ffmpeg_opt.c | 4 +---
> > >  1 file changed, 1 insertion(+), 3 deletions(-)
> > > 
> > > diff --git a/ffmpeg_opt.c b/ffmpeg_opt.c
> > > index d267c6d..1040a1c 100644
> > > --- a/ffmpeg_opt.c
> > > +++ b/ffmpeg_opt.c
> > > @@ -697,9 +697,7 @@ static void assert_file_overwrite(const char *filename)
> > >          exit_program(1);
> > >      }
> > >  
> > > -    if (!file_overwrite &&
> > > -        (strchr(filename, ':') == NULL || filename[1] == ':' ||
> > > -         av_strstart(filename, "file:", NULL))) {
> > > +    if (!file_overwrite && !strcmp(avio_find_protocol(filename), "file")) {
> > >          if (avio_check(filename, 0) == 0) {
> > >              if (stdin_interaction && !no_file_overwrite) {
> > >                  fprintf(stderr,"File '%s' already exists. Overwrite ? [y/N] ", filename);
> > 
> > Looks nice to me, thanks.
> 

>   I went for the empty string as "no protocol found" because that
> way it turns out nicer for applications where you want to directly
> compare the result of the avio_find_protocol call.
> 

>   If I return NULL to signal "no protocol found", this involves
> an extra intermediate variable and an extra NULL check.

Yes it is more work for the programmer but this will force her to
implement saner/more robust logic, since she will be forced to think
about the NULL case. Or you will have messages in the form:
"The choosen protocol is ''."

See also:
av_get_pix_fmt_name()
av_get_sample_fmt_name()
av_get_profile_name()
avcodec_get_name()
...

Whenever we return a string, the API usually returns NULL in case it
was not possible to find a corresponding string.

> Alternatively a custom string function could be used that handles
> NULL inputs.

This means usually more work and no visible gain, now you have to do a
strcmp to understand if there was no recognized element.
-- 
FFmpeg = Faithful Freak Multipurpose Power Enhancing Gangster