[FFmpeg-devel,v2,4/4] avformat/libopenmpt: Probe file format from file data if possible

Submitted by Jörn Heusipp on Jan. 10, 2018, 11:18 a.m.

Details

Message ID 1515583084-6466-5-git-send-email-osmanx@problemloesungsmaschine.de
State New
Headers show

Commit Message

Jörn Heusipp Jan. 10, 2018, 11:18 a.m.
When building with libopenmpt 0.3, use the libopenmpt file header
probing functions for probing. libopenmpt probing functions are
allocation-free and designed to be as fast as possible.

For libopenmpt 0.2, or when libopenmpt 0.3 file header probing cannot
probe successfully due to too small probe buffer, test the filename
against the file extensions supported by the libopenmpt library that
is actually linked, instead of relying on a hard-coded file extension
list. File extension testing is also allocation-free and designed to
be fast in libopenmpt. Avoiding a hard-coded file extension list is
useful because later libopenmpt versions will likely add support for
more module file formats.

libopenmpt file header probing is tested regularly against the FATE
suite and other diverse file collections by libopenmpt upstream in
order to avoid false positives.

FATE passes with './configure --enable-libopenmpt' as well as with
'./configure --enable-libopenmpt --enable-libmodplug'.

libopenmpt probing adds about 5%..10% cpu time (depending on precise
usage pattern and host CPU and compiler version used for libopenmpt)
compared to all current internal FFmpeg probing functions combined in
tools/probetest for all of its module formats combined (currently 41
modules formats in libopenmpt 0.3.4 and 234 file formats in FFmpeg).

Signed-off-by: Jörn Heusipp <osmanx@problemloesungsmaschine.de>
---
 libavformat/libopenmpt.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

Comments

Carl Eugen Hoyos Jan. 10, 2018, 11:55 a.m.
2018-01-10 12:18 GMT+01:00 Jörn Heusipp <osmanx@problemloesungsmaschine.de>:
> When building with libopenmpt 0.3, use the libopenmpt file header
> probing functions for probing. libopenmpt probing functions are
> allocation-free and designed to be as fast as possible.
>
> For libopenmpt 0.2, or when libopenmpt 0.3 file header probing cannot
> probe successfully due to too small probe buffer, test the filename
> against the file extensions supported by the libopenmpt library that
> is actually linked, instead of relying on a hard-coded file extension
> list. File extension testing is also allocation-free and designed to
> be fast in libopenmpt. Avoiding a hard-coded file extension list is
> useful because later libopenmpt versions will likely add support for
> more module file formats.
>
> libopenmpt file header probing is tested regularly against the FATE
> suite and other diverse file collections by libopenmpt upstream in
> order to avoid false positives.
>
> FATE passes with './configure --enable-libopenmpt' as well as with
> './configure --enable-libopenmpt --enable-libmodplug'.
>
> libopenmpt probing adds about 5%..10% cpu time (depending on precise
> usage pattern and host CPU and compiler version used for libopenmpt)
> compared to all current internal FFmpeg probing functions combined in
> tools/probetest for all of its module formats combined (currently 41
> modules formats in libopenmpt 0.3.4 and 234 file formats in FFmpeg).
>
> Signed-off-by: Jörn Heusipp <osmanx@problemloesungsmaschine.de>
> ---
>  libavformat/libopenmpt.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 57 insertions(+)
>
> diff --git a/libavformat/libopenmpt.c b/libavformat/libopenmpt.c
> index 5efbdc4..42b78fb 100644
> --- a/libavformat/libopenmpt.c
> +++ b/libavformat/libopenmpt.c
> @@ -218,6 +218,62 @@ static int read_seek_openmpt(AVFormatContext *s, int stream_idx, int64_t ts, int
>      return 0;
>  }
>
> +static int probe_openmpt_extension(AVProbeData *p)
> +{
> +    const char *ext;
> +    if (p->filename) {
> +        ext = strrchr(p->filename, '.');
> +        if (ext && strlen(ext + 1) > 0) {
> +            ext++;  /* skip '.' */
> +            if (openmpt_is_extension_supported(ext) == 1)
> +                return AVPROBE_SCORE_EXTENSION;
> +        }
> +    }
> +    return 0;
> +}
> +
> +static int read_probe_openmpt(AVProbeData *p)
> +{
> +#if OPENMPT_API_VERSION_AT_LEAST(0,3,0)
> +    int probe_result;
> +    if (p->buf && p->buf_size > 0) {
> +        probe_result = openmpt_probe_file_header_without_filesize(
> +                           OPENMPT_PROBE_FILE_HEADER_FLAGS_DEFAULT,
> +                           p->buf, p->buf_size,
> +                           &openmpt_logfunc, NULL, NULL, NULL, NULL, NULL);
> +        if (probe_result == OPENMPT_PROBE_FILE_HEADER_RESULT_SUCCESS) {
> +            /* As probing here relies on code external to FFmpeg, do not return
> +             * AVPROBE_SCORE_MAX in order to reduce the impact in the rare
> +             * cases of false positives.
> +             */
> +            return AVPROBE_SCORE_MIME + 1;
> +        } else if (probe_result == OPENMPT_PROBE_FILE_HEADER_RESULT_WANTMOREDATA) {

> +            if (probe_openmpt_extension(p) > 0) {
> +                return AVPROBE_SCORE_EXTENSION;

I don't think this is a good idea because it means that a high score is
reported for WANTMOREDATA.

Carl Eugen
Jörn Heusipp Jan. 10, 2018, 12:05 p.m.
On 01/10/2018 12:55 PM, Carl Eugen Hoyos wrote:
> 2018-01-10 12:18 GMT+01:00 Jörn Heusipp <osmanx@problemloesungsmaschine.de>:

>> When building with libopenmpt 0.3, use the libopenmpt file header
>> probing functions for probing. libopenmpt probing functions are
>> allocation-free and designed to be as fast as possible.
>>
>> For libopenmpt 0.2, or when libopenmpt 0.3 file header probing cannot
>> probe successfully due to too small probe buffer, test the filename
>> against the file extensions supported by the libopenmpt library that
>> is actually linked, instead of relying on a hard-coded file extension
>> list. File extension testing is also allocation-free and designed to
>> be fast in libopenmpt. Avoiding a hard-coded file extension list is
>> useful because later libopenmpt versions will likely add support for
>> more module file formats.
>>
>> libopenmpt file header probing is tested regularly against the FATE
>> suite and other diverse file collections by libopenmpt upstream in
>> order to avoid false positives.
>>
>> FATE passes with './configure --enable-libopenmpt' as well as with
>> './configure --enable-libopenmpt --enable-libmodplug'.
>>
>> libopenmpt probing adds about 5%..10% cpu time (depending on precise
>> usage pattern and host CPU and compiler version used for libopenmpt)
>> compared to all current internal FFmpeg probing functions combined in
>> tools/probetest for all of its module formats combined (currently 41
>> modules formats in libopenmpt 0.3.4 and 234 file formats in FFmpeg).
>>
>> Signed-off-by: Jörn Heusipp <osmanx@problemloesungsmaschine.de>
>> ---
>>   libavformat/libopenmpt.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 57 insertions(+)
>>
>> diff --git a/libavformat/libopenmpt.c b/libavformat/libopenmpt.c
>> index 5efbdc4..42b78fb 100644
>> --- a/libavformat/libopenmpt.c
>> +++ b/libavformat/libopenmpt.c
>> @@ -218,6 +218,62 @@ static int read_seek_openmpt(AVFormatContext *s, int stream_idx, int64_t ts, int
>>       return 0;
>>   }
>>
>> +static int probe_openmpt_extension(AVProbeData *p)
>> +{
>> +    const char *ext;
>> +    if (p->filename) {
>> +        ext = strrchr(p->filename, '.');
>> +        if (ext && strlen(ext + 1) > 0) {
>> +            ext++;  /* skip '.' */
>> +            if (openmpt_is_extension_supported(ext) == 1)
>> +                return AVPROBE_SCORE_EXTENSION;
>> +        }
>> +    }
>> +    return 0;
>> +}
>> +
>> +static int read_probe_openmpt(AVProbeData *p)
>> +{
>> +#if OPENMPT_API_VERSION_AT_LEAST(0,3,0)
>> +    int probe_result;
>> +    if (p->buf && p->buf_size > 0) {
>> +        probe_result = openmpt_probe_file_header_without_filesize(
>> +                           OPENMPT_PROBE_FILE_HEADER_FLAGS_DEFAULT,
>> +                           p->buf, p->buf_size,
>> +                           &openmpt_logfunc, NULL, NULL, NULL, NULL, NULL);
>> +        if (probe_result == OPENMPT_PROBE_FILE_HEADER_RESULT_SUCCESS) {
>> +            /* As probing here relies on code external to FFmpeg, do not return
>> +             * AVPROBE_SCORE_MAX in order to reduce the impact in the rare
>> +             * cases of false positives.
>> +             */
>> +            return AVPROBE_SCORE_MIME + 1;
>> +        } else if (probe_result == OPENMPT_PROBE_FILE_HEADER_RESULT_WANTMOREDATA) {
> 
>> +            if (probe_openmpt_extension(p) > 0) {
>> +                return AVPROBE_SCORE_EXTENSION;
> 
> I don't think this is a good idea because it means that a high score is
> reported for WANTMOREDATA.

That is what the first patch did also, however much more convoluted and 
not easy to spot.
In any case, I agree, that does not appear like that good of an idea. 
Will change to AVPROBE_SCORE_RETRY with the next iteration of the patch.


Regards,
Jörn

Patch hide | download patch | download mbox

diff --git a/libavformat/libopenmpt.c b/libavformat/libopenmpt.c
index 5efbdc4..42b78fb 100644
--- a/libavformat/libopenmpt.c
+++ b/libavformat/libopenmpt.c
@@ -218,6 +218,62 @@  static int read_seek_openmpt(AVFormatContext *s, int stream_idx, int64_t ts, int
     return 0;
 }
 
+static int probe_openmpt_extension(AVProbeData *p)
+{
+    const char *ext;
+    if (p->filename) {
+        ext = strrchr(p->filename, '.');
+        if (ext && strlen(ext + 1) > 0) {
+            ext++;  /* skip '.' */
+            if (openmpt_is_extension_supported(ext) == 1)
+                return AVPROBE_SCORE_EXTENSION;
+        }
+    }
+    return 0;
+}
+
+static int read_probe_openmpt(AVProbeData *p)
+{
+#if OPENMPT_API_VERSION_AT_LEAST(0,3,0)
+    int probe_result;
+    if (p->buf && p->buf_size > 0) {
+        probe_result = openmpt_probe_file_header_without_filesize(
+                           OPENMPT_PROBE_FILE_HEADER_FLAGS_DEFAULT,
+                           p->buf, p->buf_size,
+                           &openmpt_logfunc, NULL, NULL, NULL, NULL, NULL);
+        if (probe_result == OPENMPT_PROBE_FILE_HEADER_RESULT_SUCCESS) {
+            /* As probing here relies on code external to FFmpeg, do not return
+             * AVPROBE_SCORE_MAX in order to reduce the impact in the rare
+             * cases of false positives.
+             */
+            return AVPROBE_SCORE_MIME + 1;
+        } else if (probe_result == OPENMPT_PROBE_FILE_HEADER_RESULT_WANTMOREDATA) {
+            if (probe_openmpt_extension(p) > 0) {
+                return AVPROBE_SCORE_EXTENSION;
+            } else {
+                if (p->buf_size >= openmpt_probe_file_header_get_recommended_size()) {
+                    /* We have already received the recommended amount of data
+                     * and still cannot decide. Return a rather low score.
+                     */
+                    return AVPROBE_SCORE_RETRY / 2;
+                } else {
+                    /* The file extension is unknown and we have very few data
+                     * bytes available. libopenmpt cannot decide anything here,
+                     * and returning any score > 0 would result in successfull
+                     * probing of random data.
+                     */
+                    return 0;
+                }
+            }
+        } else if (probe_result == OPENMPT_PROBE_FILE_HEADER_RESULT_FAILURE) {
+            return 0;
+        }
+    }
+#endif
+    /* for older libopenmpt, fall back to file extension probing */
+    return probe_openmpt_extension(p);
+}
+
 static const AVClass class_openmpt = {
     .class_name = "libopenmpt",
     .item_name  = av_default_item_name,
@@ -229,6 +285,7 @@  AVInputFormat ff_libopenmpt_demuxer = {
     .name           = "libopenmpt",
     .long_name      = NULL_IF_CONFIG_SMALL("Tracker formats (libopenmpt)"),
     .priv_data_size = sizeof(OpenMPTContext),
+    .read_probe     = read_probe_openmpt,
     .read_header    = read_header_openmpt,
     .read_packet    = read_packet_openmpt,
     .read_close     = read_close_openmpt,