[FFmpeg-devel] Is av_log_level supposed to be exported?

[Michael Niedermayer]

On Thu, Dec 03, 2009 at 01:58:56PM -0200, Ramiro Polla wrote:
> On Thu, Dec 3, 2009 at 12:03 PM, Diego Biurrun <diego at biurrun.de> wrote:
> > On Thu, Dec 03, 2009 at 11:33:06AM -0200, Ramiro Polla wrote:
> >> On Thu, Dec 3, 2009 at 9:14 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> >> > On Thu, Dec 03, 2009 at 09:52:44AM +0100, Diego Biurrun wrote:
> >> >> On Thu, Dec 03, 2009 at 04:47:28AM +0100, Michael Niedermayer wrote:
> >> >> > On Wed, Dec 02, 2009 at 10:42:36PM -0200, Ramiro Polla wrote:
> >> >> > >
> >> >> > > av_log_level from libavutil/log.c is only used in that file. It is not
> >> >> > > declared in libavutil/log.h. It's meant to be used with the
> >> >> > > av_log_get_level() and av_log_set_level() functions.
> >> >> > >
> >> >> > > Should it be made static in log.c, or declared in libavutil/log.h so
> >> >> > > users can access it directly?
> >> >> >
> >> >> > should be made static in the next maj ver bump
> >> >>
> >> >> Why care about API abuse? ?I'd just make it static right away...
> >> >
> >> > IIRC it was once the only way to set the field, so some appliations
> >> > might still use it.
> >>
> >> I think it was already supposed to be gone by version 50.
> >>
> >> -------------
> >> Author: lucabe
> >> Date: Thu Feb ?8 19:55:36 2007
> >> New Revision: 7882
> >>
> >> Modified:
> >> ? ?trunk/ffmpeg.c
> >> ? ?trunk/ffplay.c
> >> ? ?trunk/libavcodec/utils.c
> >>
> >> Log:
> >> Directly access av_log_level instead of calling av_log_{set, get}_level()
> >> (which will be removed at the next avutil version increment)
> >> [...]
> >> -------------
> >> Author: mru
> >> Date: Wed Dec 12 22:48:50 2007
> >> New Revision: 11209
> >>
> >> Log:
> >> use av_log_get/set_level()
> >>
> >> Modified:
> >> ? ?trunk/ffmpeg.c
> >> ? ?trunk/ffplay.c
> >> ? ?trunk/libavcodec/utils.c
> >> ? ?trunk/libavutil/log.c
> >> ? ?trunk/libavutil/log.h
> >>
> >> [...]
> >> --- trunk/libavutil/log.h ? ? (original)
> >> +++ trunk/libavutil/log.h ? ? Wed Dec 12 22:48:50 2007
> >> @@ -81,7 +81,10 @@ struct AVCLASS {
> >> ? */
> >> ?#define AV_LOG_DEBUG ? ?48
> >> ?#endif
> >> +
> >> +#if LIBAVUTIL_VERSION_INT < (50<<16)
> >> ?extern int av_log_level;
> >> +#endif
> >> --------------------
> >>
> >> It just wasn't made conditionally static in r11209, so I think Diego
> >> might be right...
> >
> > Diego says: Just make it static :)
> 
> And what does Simon says? =) If there are no more comments I'll just
> make it static...

I say make it static during the next major bump.

yes, applications _should_not_ use it but there was no warning for them
to update to the new ABI and the old is still working. Making it static
has a high chance of breaking applications. Distro maintainers will
lynch you if any application used it as it will break that app ...


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

If a bugfix only changes things apparently unrelated to the bug with no
further explanation, that is a good sign that the bugfix is wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091203/8debaa0a/attachment.pgp>