[FFmpeg-devel] [FFmpeg-cvslog] doc: document the addition of the AVProbeData.mime_type field and it' s implications

Michael Niedermayer michaelni at gmx.at
Sun Sep 14 00:43:45 CEST 2014


On Sun, Sep 14, 2014 at 12:22:02AM +0200, Andreas Cadhalpun wrote:
> On 13.09.2014 23:54, Clément Bœsch wrote:
> >On Sat, Sep 13, 2014 at 11:38:56PM +0200, Andreas Cadhalpun wrote:
> >>On 13.09.2014 15:25, Michael Niedermayer wrote:
> >>>On Sat, Sep 13, 2014 at 08:24:39AM +0200, Clément Bœsch wrote:
> >>>>On Sat, Sep 13, 2014 at 12:53:21AM +0200, Andreas Cadhalpun wrote:
> >>>>>ffmpeg | branch: master | Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com> | Fri Sep 12 18:18:42 2014 +0200| [d5e802609a0046441798cdbd137c96e4aa912390] | committer: Michael Niedermayer
> >>>>>
> >>>>>doc: document the addition of the AVProbeData.mime_type field and it's implications
> >>>>>
> >>>>>Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >>>>>Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> >>>>>
> >>>>>>http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=d5e802609a0046441798cdbd137c96e4aa912390
> >>>>>---
> >>>>>
> >>>>>  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;'.
> >>>>
> >>>>I don't think we should mix API and UI in this file. The second sentence
> >>>>can be moved to doc/APIchanges, that's what this file is used for. We
> >>>>mention doc/APIchanges in that RELEASE_NOTES: "Please refer to the
> >>>>doc/APIChanges file for more information."
> >>
> >
> >>Indeed, that is a good idea. But still the other sentence doesn't really fit
> >>into the 'Behaviour changes' section, so I moved it to the 'API Information'
> >>section, where it fits much better and is directly followed by the pointer
> >>to doc/APIChanges.
> >
> >I disagree, there is no reason to make an exception for that.
> 
> It would be good to mention this explicitly in the RELEASE_NOTES,
> because otherwise it just says the API didn't change and then people
> will be surprised that their programs crash.
> 
> >>In the attached patch I also changed the text at the beginning to only claim
> >>that the API is mostly compatible and minimal source changes might be
> >>needed.
> >>
> >>Best regards,
> >>Andreas
> >
> >> From 8170dc6151199bda655b3726f4cd129abe7781e1 Mon Sep 17 00:00:00 2001
> >>From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >>Date: Sat, 13 Sep 2014 23:34:09 +0200
> >>Subject: [PATCH] doc: don't mix API and UI changes in the 'Behaviour changes'
> >>  section of the RELEASE_NOTES
> >>
> >>Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >>---
> >>  RELEASE_NOTES  | 10 +++++-----
> >>  doc/APIchanges |  2 ++
> >>  2 files changed, 7 insertions(+), 5 deletions(-)
> >>
> >>diff --git a/RELEASE_NOTES b/RELEASE_NOTES
> >>index 14513a7..6c26aff 100644
> >>--- a/RELEASE_NOTES
> >>+++ b/RELEASE_NOTES
> >>@@ -3,8 +3,8 @@
> >>   ???????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????????
> >>
> >
> >You have utf-8 troubles, the diff won't apply. Try git-send-email maybe.
> 
> Have you tried saving the attachement? It seems the last one did work...
> 
> >>     The FFmpeg Project proudly presents FFmpeg <next> "FIXME", ...
> >>-   FFmpeg 2.4 is API-, but not ABI-compatible with the previous major release.
> >>-   This means that the code using our libraries needs to be rebuilt, but no
> >>+   FFmpeg 2.4 is mostly API-, but not ABI-compatible with the previous major release.
> >>+   This means that the code using our libraries needs to be rebuilt, but only minimal
> >>     source changes should be required.
> >
> >Unrelated
> 
> In a way it's related, since this is about the AVProbeData.mime_type
> API change, and it is confusing to state that the API didn't change,
> when it did.
> But I can send a separate patch for that, if that's preferred.
> 
> >>
> >>     ??????????????????????????????????????????????????????????????????????????????????????????
> >>@@ -22,6 +22,9 @@
> >>         ??? libswresample  xx.yy.1zz
> >>         ??? libpostproc    xx.yy.1zz
> >
> >>
> >>+     IMPORTANT: The new field mime_type was added to AVProbeData, which can
> >>+     cause crashes, if it is not initialized.
> >>+
> >
> >No, please just drop it.
> 
> Why don't you want to mention this here?
> 
> >>       Please refer to the doc/APIChanges file for more information.
> >>
> >>   ??????????????????????????????????????????????????????????????????????????????????????????
> >>@@ -54,9 +57,6 @@
> >>   ??? ???  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 90048a5..e3d402d 100644
> >>--- a/doc/APIchanges
> >>+++ b/doc/APIchanges
> >>@@ -97,6 +97,8 @@ API changes, most recent first:
> >>
> >>  2014-07-29 - 80a3a66 / 3a19405 - lavf 56.01.100 / 56.01.0 - avformat.h
> >>    Add mime_type field to AVProbeData.
> >>+  To avoid crashes, make sure to always initialize AVProbeData, e.g. use
> >>+
> >
> >I suggest the following:
> >   Add mime_type field to AVProbeData, which now MUST be initialized in
> >   order to avoid uninitialized reads of the mime_type pointer, likely
> >   leading to crashes.
> >   Typically, this means you will do 'AVProbeData pd = { 0 };' instead of
> >   'AVProbeData pd;'.
> 
> This wording is fine.
> 
> >Maybe we will want to make a clear cut in the doc/APIChanges file to
> >separate the releases, so this change will not be missed.
> 
> Yes, that might help.

note, i will release 2.4 tomorrow (2014-09-14) or rather i intend
to. So anything that isnt pushed to the RELEASE_NOTES by then wont
be in it

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Old school: Use the lowest level language in which you can solve the problem
            conveniently.
New school: Use the highest level language in which the latest supercomputer
            can solve the problem without the user falling asleep waiting.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140914/1daa5378/attachment.asc>


More information about the ffmpeg-devel mailing list