[FFmpeg-devel] default lock mechanism in libavcodec/utils.c

Michael Niedermayer michaelni at gmx.at
Thu Mar 6 18:07:17 CET 2014


On Thu, Mar 06, 2014 at 02:41:34PM +0530, anshul wrote:
> On 03/01/2014 02:57 PM, anshul wrote:
> >On 02/19/2014 01:52 PM, anshul wrote:
> >>On 02/18/2014 10:02 PM, Michael Niedermayer wrote:
> >>>On Wed, Jan 29, 2014 at 11:49:41AM +0530, anshul wrote:
> >>>>On 01/29/2014 11:11 AM, anshul wrote:
> >>>>>On 01/28/2014 08:31 PM, Michael Niedermayer wrote:
> >>>>>>if you had avcodec_unregister_all() you will have to deal with
> >>>>>>libgstreamer and libvlc calling it while your app is also
> >>>>>>calling it and maybe another lib calling register and yet another
> >>>>>>still using avcodec. all happening at the same time
> >>>>>>
> >>>>>>this can be solved in various ways, a lock and reference counting is
> >>>>>>one, another is to use some code that gets called on exit or
> >>>>>>lib unloading by the OS.
> >>>>>>I think none of these will be really easy to do portably
> >>>>>If I am planning to use some code that gets called when library is
> >>>>>unloaded.
> >>>>>I do need static avcodec lock acess out of library, I have
> >>>>>attached one patch
> >>>>>so that user might be able to call this function in his destructor
> >>>>>of library
> >>>>>
> >>>>>But your idea about reference count is very good, but I don't
> >>>>>where to keep that
> >>>>>reference count variable, only thing that strike my mind was to
> >>>>>make reference count variable global.
> >>>>>If you have any other way then making it global please ...
> >>>>>
> >>>>>
> >>>>>Thanks
> >>>>>Anshul
> >>>>>
> >>>>>
> >>>>forgot to add return 0; my new patch with same thing
> >>>>
> >>>>Thanks
> >>>>Anshul Maheshwari
> >>>>  internal.h |    1 +
> >>>>  utils.c    |    8 ++++++++
> >>>>  2 files changed, 9 insertions(+)
> >>>>47599b4e3f49bde9a68d3e398958472e4b18b61f  0001-added-ff_destroy_lock_avcodec-so-that-it-can-be-call.patch
> >>>> From 731dd24ba9b138f626baa9908fdb67f74ef64fe5 Mon Sep 17 00:00:00 2001
> >>>>From: Anshul Maheshwari<er.anshul.maheshwari at gmail.com>
> >>>>Date: Wed, 29 Jan 2014 11:39:11 +0530
> >>>>Subject: [PATCH] added ff_destroy_lock_avcodec so that it can be called by
> >>>>  user if they want to free the mutex
> >>>>
> >>>>---
> >>>>  libavcodec/internal.h | 1 +
> >>>>  libavcodec/utils.c    | 8 ++++++++
> >>>>  2 files changed, 9 insertions(+)
> >>>>
> >>>>diff --git a/libavcodec/internal.h b/libavcodec/internal.h
> >>>>index 8aa0ac1..2c85b7b 100644
> >>>>--- a/libavcodec/internal.h
> >>>>+++ b/libavcodec/internal.h
> >>>>@@ -151,6 +151,7 @@ void avpriv_color_frame(AVFrame *frame, const int color[4]);
> >>>>  extern volatile int ff_avcodec_locked;
> >>>>  int ff_lock_avcodec(AVCodecContext *log_ctx);
> >>>>  int ff_unlock_avcodec(void);
> >>>>+int ff_destroy_lock_avcodec(void);
> >>>>  int avpriv_lock_avformat(void);
> >>>>  int avpriv_unlock_avformat(void);
> >>>>diff --git a/libavcodec/utils.c b/libavcodec/utils.c
> >>>>index c8fd8c6..20cc00b 100644
> >>>>--- a/libavcodec/utils.c
> >>>>+++ b/libavcodec/utils.c
> >>>>@@ -3318,6 +3318,14 @@ int ff_unlock_avcodec(void)
> >>>>      return 0;
> >>>>  }
> >>>>+int ff_destroy_lock_avcodec(void)
> >>>>+{
> >>>>+    if(lockmgr_cb) {
> >>>>+        if (lockmgr_cb(&codec_mutex, AV_LOCK_DESTROY))
> >>>>+            return -1;
> >>>>+    }
> >>>>+    return 0;
> >>>>+}
> >>>ff_ prefixed functions are internal  and considering its in internal.h
> >>>maybe thats what you intended but nothing calls it so this is just
> >>>unused and undocumneted code.
> >>>and if it wasnt internal people wouldnt know when or from where to
> >>>call this without documentation
> >>>
> >>>also the function name should make it clear that it is unsafe to be
> >>>called when anything is still running or might use the lib afterwards
> >>>maybe
> >>>av_cleanup_on_lib_unload()
> >>>would be a better name for that ...
> >>>
> >>>
> >>>[...]
> >>>
> >>>
> >>>
> >>>_______________________________________________
> >>>ffmpeg-devel mailing list
> >>>ffmpeg-devel at ffmpeg.org
> >>>http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >>New Patch is attached
> >>
> >>I have done some doxygen kind of Documentation for function
> >>av_cleanup_on_lib_unload()
> >>I have called ff_destroy_lock_avcodec inside
> >>av_cleanup_on_lib_unload, since I am still looking for
> >>reference counting idea over there i would require function
> >>ff_destroy_lock_avcodec.
> >>
> >>If anyone know some good example application where it use or
> >>require the libavcodec locks are welcome, i am unable to
> >>undestand the lock mecanism atleast with ffmpeg.
> >>
> >>If anyone know of any utility which show locking
> >>diagrammatically because I have a fear that I would miss some
> >>scenario while considering all.
> >>
> >>Thanks
> >>Anshul Maheshwari
> >ping
> ping

it appears there are multiple people in this thread who are against
this patch

also its documentation is incomplete, it does not state when it is safe
and when it is not safe to use. And i suspect once you completely
document that and read this documentation you would agree that this
function is quite easy to cause serious bugs and very hard
to use saftely.

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

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140306/d041f047/attachment.asc>


More information about the ffmpeg-devel mailing list