diff mbox series

[FFmpeg-devel,1/5] avformat/format: add av_find_input_format2

Message ID 20200128074446.7546-1-ffmpeg@gyani.pro
State New
Headers show
Series [FFmpeg-devel,1/5] avformat/format: add av_find_input_format2 | expand

Checks

Context Check Description
andriy/ffmpeg-patchwork success Make fate finished

Commit Message

Gyan Doshi Jan. 28, 2020, 7:44 a.m. UTC 
Identifies demuxer by extension if search by short name fails.
---
 libavformat/avformat.h |  7 +++++++
 libavformat/format.c   | 14 +++++++++++++-
 libavformat/version.h  |  2 +-
 3 files changed, 21 insertions(+), 2 deletions(-)

Comments

Andreas Rheinhardt Jan. 28, 2020, 2:38 p.m. UTC | #1 
Gyan Doshi:
> Identifies demuxer by extension if search by short name fails.
> ---
>  libavformat/avformat.h |  7 +++++++
>  libavformat/format.c   | 14 +++++++++++++-
>  libavformat/version.h  |  2 +-
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 9b9b634ec3..c81c4f18fd 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -2246,6 +2246,13 @@ ff_const59 AVInputFormat *av_find_input_format(const char *short_name);
>   */
>  ff_const59 AVInputFormat *av_probe_input_format(ff_const59 AVProbeData *pd, int is_opened);
>  
> +/**
> + * Find AVInputFormat based on the short name of the input format.
> + * If that fails and as_extension is set, find demuxer which has registered the
> + * name as an extension.
> + */
> +ff_const59 AVInputFormat *av_find_input_format2(const char *short_name, int as_extension);
> +
There is no reason not to make this function const-correct from the
beginning, i.e. replace ff_const59 by const.
>  /**
>   * Guess the file format.
>   *
> diff --git a/libavformat/format.c b/libavformat/format.c
> index c47490c8eb..d2382d1cd0 100644
> --- a/libavformat/format.c
> +++ b/libavformat/format.c
> @@ -115,16 +115,28 @@ enum AVCodecID av_guess_codec(ff_const59 AVOutputFormat *fmt, const char *short_
>          return AV_CODEC_ID_NONE;
>  }
>  
> -ff_const59 AVInputFormat *av_find_input_format(const char *short_name)
> +ff_const59 AVInputFormat *av_find_input_format2(const char *short_name, int as_extension)
>  {
>      const AVInputFormat *fmt = NULL;
>      void *i = 0;
>      while ((fmt = av_demuxer_iterate(&i)))
>          if (av_match_name(short_name, fmt->name))
>              return (AVInputFormat*)fmt;

Then you don't need to cast the const away here

> +
> +    if (as_extension) {
> +        i = 0;
> +        while ((fmt = av_demuxer_iterate(&i)))
> +            if (fmt->extensions && av_match_name(short_name, fmt->extensions))
> +                return (AVInputFormat*)fmt;

and here.

> +    }
>      return NULL;
>  }
>  
> +ff_const59 AVInputFormat *av_find_input_format(const char *short_name)
> +{
> +    return av_find_input_format2(short_name, 0);

But you will have to cast the const away here.

Not commenting on whether this function should be added at all.

- Andreas
Anton Khirnov Jan. 29, 2020, 9:40 a.m. UTC | #2 
Quoting Gyan Doshi (2020-01-28 08:44:42)
> Identifies demuxer by extension if search by short name fails.
> ---
>  libavformat/avformat.h |  7 +++++++
>  libavformat/format.c   | 14 +++++++++++++-
>  libavformat/version.h  |  2 +-
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> index 9b9b634ec3..c81c4f18fd 100644
> --- a/libavformat/avformat.h
> +++ b/libavformat/avformat.h
> @@ -2246,6 +2246,13 @@ ff_const59 AVInputFormat *av_find_input_format(const char *short_name);
>   */
>  ff_const59 AVInputFormat *av_probe_input_format(ff_const59 AVProbeData *pd, int is_opened);
>  
> +/**
> + * Find AVInputFormat based on the short name of the input format.
> + * If that fails and as_extension is set, find demuxer which has registered the
> + * name as an extension.
> + */
> +ff_const59 AVInputFormat *av_find_input_format2(const char *short_name, int as_extension);

This seems rather ad-hoc to me. I think it'd be cleaner to have a
dedicated function just for matching by extension. It could be called
av_demuxer_find_by_ext() for consistency with av_demuxer_iterate().
Gyan Doshi Jan. 29, 2020, 3:48 p.m. UTC | #3 
On 29-01-2020 03:10 pm, Anton Khirnov wrote:
> Quoting Gyan Doshi (2020-01-28 08:44:42)
>> Identifies demuxer by extension if search by short name fails.
>> ---
>>   libavformat/avformat.h |  7 +++++++
>>   libavformat/format.c   | 14 +++++++++++++-
>>   libavformat/version.h  |  2 +-
>>   3 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
>> index 9b9b634ec3..c81c4f18fd 100644
>> --- a/libavformat/avformat.h
>> +++ b/libavformat/avformat.h
>> @@ -2246,6 +2246,13 @@ ff_const59 AVInputFormat *av_find_input_format(const char *short_name);
>>    */
>>   ff_const59 AVInputFormat *av_probe_input_format(ff_const59 AVProbeData *pd, int is_opened);
>>   
>> +/**
>> + * Find AVInputFormat based on the short name of the input format.
>> + * If that fails and as_extension is set, find demuxer which has registered the
>> + * name as an extension.
>> + */
>> +ff_const59 AVInputFormat *av_find_input_format2(const char *short_name, int as_extension);
> This seems rather ad-hoc to me. I think it'd be cleaner to have a
> dedicated function just for matching by extension. It could be called
> av_demuxer_find_by_ext() for consistency with av_demuxer_iterate().

Sure.  But it could be both.  av_find_input_format2 could call 
av_find_format_by_ext() if name fails.

Gyan
Anton Khirnov Jan. 30, 2020, 3:16 p.m. UTC | #4 
Quoting Gyan (2020-01-29 16:48:14)
> 
> 
> On 29-01-2020 03:10 pm, Anton Khirnov wrote:
> > Quoting Gyan Doshi (2020-01-28 08:44:42)
> >> Identifies demuxer by extension if search by short name fails.
> >> ---
> >>   libavformat/avformat.h |  7 +++++++
> >>   libavformat/format.c   | 14 +++++++++++++-
> >>   libavformat/version.h  |  2 +-
> >>   3 files changed, 21 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/libavformat/avformat.h b/libavformat/avformat.h
> >> index 9b9b634ec3..c81c4f18fd 100644
> >> --- a/libavformat/avformat.h
> >> +++ b/libavformat/avformat.h
> >> @@ -2246,6 +2246,13 @@ ff_const59 AVInputFormat *av_find_input_format(const char *short_name);
> >>    */
> >>   ff_const59 AVInputFormat *av_probe_input_format(ff_const59 AVProbeData *pd, int is_opened);
> >>   
> >> +/**
> >> + * Find AVInputFormat based on the short name of the input format.
> >> + * If that fails and as_extension is set, find demuxer which has registered the
> >> + * name as an extension.
> >> + */
> >> +ff_const59 AVInputFormat *av_find_input_format2(const char *short_name, int as_extension);
> > This seems rather ad-hoc to me. I think it'd be cleaner to have a
> > dedicated function just for matching by extension. It could be called
> > av_demuxer_find_by_ext() for consistency with av_demuxer_iterate().
> 
> Sure.  But it could be both.  av_find_input_format2 could call 
> av_find_format_by_ext() if name fails.

Possible, yes. Though I'd suggest 'guess' rather than 'find' for a
function that does "get me some input format based on some heuristics".

Also, I really think it's better to use big-endian naming along the
lines of av[_]<object>_<action> rather than av[_]_<do_stuff>.
diff mbox series

Patch

diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index 9b9b634ec3..c81c4f18fd 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -2246,6 +2246,13 @@  ff_const59 AVInputFormat *av_find_input_format(const char *short_name);
  */
 ff_const59 AVInputFormat *av_probe_input_format(ff_const59 AVProbeData *pd, int is_opened);
 
+/**
+ * Find AVInputFormat based on the short name of the input format.
+ * If that fails and as_extension is set, find demuxer which has registered the
+ * name as an extension.
+ */
+ff_const59 AVInputFormat *av_find_input_format2(const char *short_name, int as_extension);
+
 /**
  * Guess the file format.
  *
diff --git a/libavformat/format.c b/libavformat/format.c
index c47490c8eb..d2382d1cd0 100644
--- a/libavformat/format.c
+++ b/libavformat/format.c
@@ -115,16 +115,28 @@  enum AVCodecID av_guess_codec(ff_const59 AVOutputFormat *fmt, const char *short_
         return AV_CODEC_ID_NONE;
 }
 
-ff_const59 AVInputFormat *av_find_input_format(const char *short_name)
+ff_const59 AVInputFormat *av_find_input_format2(const char *short_name, int as_extension)
 {
     const AVInputFormat *fmt = NULL;
     void *i = 0;
     while ((fmt = av_demuxer_iterate(&i)))
         if (av_match_name(short_name, fmt->name))
             return (AVInputFormat*)fmt;
+
+    if (as_extension) {
+        i = 0;
+        while ((fmt = av_demuxer_iterate(&i)))
+            if (fmt->extensions && av_match_name(short_name, fmt->extensions))
+                return (AVInputFormat*)fmt;
+    }
     return NULL;
 }
 
+ff_const59 AVInputFormat *av_find_input_format(const char *short_name)
+{
+    return av_find_input_format2(short_name, 0);
+}
+
 ff_const59 AVInputFormat *av_probe_input_format3(ff_const59 AVProbeData *pd, int is_opened,
                                       int *score_ret)
 {
diff --git a/libavformat/version.h b/libavformat/version.h
index f72fb9478a..15fdb73e3e 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@ 
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  58
-#define LIBAVFORMAT_VERSION_MINOR  36
+#define LIBAVFORMAT_VERSION_MINOR  37
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \