On Fri, 12 Sep 2014 18:51:48 +0200 Andreas Cadhalpun <andreas.cadhal...@googlemail.com> wrote:
> 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]. I'm not very sure; note that AVPROBE_SCORE_EXTENSION does NOT refer to the probe score set if the extension matches. This is a relict from the past, and actual extension probe scores are much lower. But I guess AVPROBE_SCORE_MIME was indeed to be used this way. > >> 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 _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel