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

[Anshul]

Michael Niedermayer <michaelni at gmx.at> wrote:
>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.
>
>[...]
Thanks i drop the idea
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.