[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