Hi,
On 12.09.2014 15:18, Michael Niedermayer wrote:
On Fri, Sep 12, 2014 at 01:54:36PM +0200, Andreas Cadhalpun wrote:
On 11.08.2014 22:22, Michael Niedermayer wrote:
On Mon, Aug 11, 2014 at 08:05:38PM +0200, Reimar Döffinger wrote:
Hello,
(sorry for being too lazy to send a patch)
With the major version bump AVProbeData was extended by a new field.
This so far has broken 3 places within FFmpeg and one within MPlayer,
where AVProbeData was only initialized field-by-field.
This will cause "random" crashes.
I'm at this point fairly certain a lot of other software will have the
same issue.
That's for sure.
I suggest we make add a big note with the release that everyone should
check their software for uses of AVProbeData that might result in parts
of that struct not being initialized.
agree
Please really document this!
Attached 0001 patch adds documentation for this.
The 0002 patch changes the score to AVPROBE_SCORE_MIME for matching mime
types, which seems to have been intended [0].
It broke mpd [1] and the mpd developer wasn't happy that this is not
documented [2].
But there is also another problem, as can be seen from the comment
in the fix [3]:
/* this attribute was added in libav/ffmpeg version 11, but
unfortunately it's "uint8_t" instead of "char", and it's
not "const" - wtf? */
I'm also wondering, why AVProbeData.mime_type is a uint8_t* instead
of a const char*.
dont ask me
Well, then I'm CC'ing the author of this commit.
This was introduced in commit
3a19405d574a467c68b48e4b824c76617fd59de0 (merged from Libav) and in
the same commit lpd.mime_type is used as argument for av_match_name,
which takes const char* ...
And mime_type was added as const char* to AVInputFormat, so this is
even inconsistent.
So should this be changed to const char*?
i would tend toward a yes.
Though in theory this could lead to software that builds with libav
and doesnt with ffmpeg
for example if someone assigns a array to pd.mime_type and then
writes into that by using pd.mime_type
Not only in theory, because if one tries to assign a uint8_t* to a const
char* variable or vice versa, compilation will fail with e.g.:
error: invalid conversion from ‘uint8_t* {aka unsigned char*}’ to ‘const
char*’ [-fpermissive]
its really more a question what the community prefers here
100% compatibility or 99% and the more sane type
Thus I'd say that in order to retain compatibility the 0003 patch,
changing AVProbeData.mime_type from uint8_t* to const char*, should only
be applied, if this change is also applied in Libav, even though I think
it is wrong the way it is now.
Best regards,
Andreas
0: https://lists.libav.org/pipermail/libav-devel/2014-July/061038.html
>From 8869df5a02654a56b048a3abebc5ad838c2185e7 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Fri, 12 Sep 2014 18:18:42 +0200
Subject: [PATCH 1/3] doc: document the addition of the AVProbeData.mime_type
field and it's implications
Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
RELEASE_NOTES | 3 +++
doc/APIchanges | 3 +++
2 files changed, 6 insertions(+)
diff --git a/RELEASE_NOTES b/RELEASE_NOTES
index 113cc5e..14513a7 100644
--- a/RELEASE_NOTES
+++ b/RELEASE_NOTES
@@ -54,6 +54,9 @@
â â Behaviour changes â
ââââââââââââââââââââââââââââââ
+ ⢠IMPORTANT: The new field mime_type was added to AVProbeData.
+ To avoid crashes, make sure to always initialize AVProbeData, e.g. use
+ 'AVProbeData pd = { 0 };' instead of 'AVProbeData pd;'.
⢠dctdnoiz filter now uses a block size of 8x8 instead of 16x16 by default
⢠-vismv option is deprecated in favor of the codecview filter
⢠libmodplug is now detected through pkg-config
diff --git a/doc/APIchanges b/doc/APIchanges
index 6d214ef..90048a5 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -95,6 +95,9 @@ API changes, most recent first:
2014-07-30 - ba3e331 - lavu 52.94.100 - frame.h
Add av_frame_side_data_name()
+2014-07-29 - 80a3a66 / 3a19405 - lavf 56.01.100 / 56.01.0 - avformat.h
+ Add mime_type field to AVProbeData.
+
2014-07-29 - 31e0b5d / 69e7336 - lavu 52.92.100 / 53.19.0 - avstring.h
Make name matching function from lavf public as av_match_name().
--
2.1.0
>From b3ef6e80290337f09da8bd42e31d3bf7acf4eaeb Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Fri, 12 Sep 2014 18:20:08 +0200
Subject: [PATCH 2/3] lavf/format.c: use AVPROBE_SCORE_MIME instead of
AVPROBE_SCORE_EXTENSION for matching mime types
Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
libavformat/format.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/format.c b/libavformat/format.c
index 828ab52..4bca384 100644
--- a/libavformat/format.c
+++ b/libavformat/format.c
@@ -217,7 +217,7 @@ AVInputFormat *av_probe_input_format3(AVProbeData *pd, int is_opened,
score = AVPROBE_SCORE_EXTENSION;
}
if (av_match_name(lpd.mime_type, fmt1->mime_type))
- score = FFMAX(score, AVPROBE_SCORE_EXTENSION);
+ score = FFMAX(score, AVPROBE_SCORE_MIME);
if (score > score_max) {
score_max = score;
fmt = fmt1;
--
2.1.0
>From f5cbadb1f821accfd786695f65297faff0d35e62 Mon Sep 17 00:00:00 2001
From: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
Date: Fri, 12 Sep 2014 18:22:03 +0200
Subject: [PATCH 3/3] lavf/avformat.h: use const char* instead of uint8_t* for
AVProbeData.mime_type
This makes the field consistent with AVInputFormat.mime_type and the argument type of av_match_name.
Signed-off-by: Andreas Cadhalpun <andreas.cadhal...@googlemail.com>
---
libavformat/avformat.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libavformat/avformat.h b/libavformat/avformat.h
index b915148..1ddfc9a 100644
--- a/libavformat/avformat.h
+++ b/libavformat/avformat.h
@@ -398,7 +398,7 @@ typedef struct AVProbeData {
const char *filename;
unsigned char *buf; /**< Buffer must have AVPROBE_PADDING_SIZE of extra allocated bytes filled with zero. */
int buf_size; /**< Size of buf except extra allocated bytes */
- uint8_t *mime_type; /**< mime_type, when known. */
+ const char *mime_type; /**< mime_type, when known. */
} AVProbeData;
#define AVPROBE_SCORE_RETRY (AVPROBE_SCORE_MAX/4)
--
2.1.0
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
