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

[Clément Bœsch]

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.

> 
> 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.

>     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

>  
>     ??????????????????????????????????????????????????????????????????????????????????????????
> @@ -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.

>       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;'.

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.

[...]

-- 
Clément B.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140913/7a0467e7/attachment.asc>