[FFmpeg-devel] [PATCH] avformat: avisynth demuxer rewrite

[d s]

On Mon, Sep 24, 2012 at 12:21 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
>> This bothers me too, but I'm not sure how to approach this. I don't
>> want to construct the mutex unless the demuxer is actually being used,
>> but if I don't have an atomic operation somewhere, there could be a
>> race condition calling pthread_mutex_init.
>
> maybe you can use avpriv_lock_avformat() and avpriv_unlock_avformat()
> instead of this or the whole mutex
>
I read the code, and it seems the user must create a lock manager,
without which those functions are NOPs. Is it safe to assume that
ff_lockmgr_cb will exist in every situation where multiple avformat
threads are running?


>> >>                 avs_mutex = av_malloc(sizeof(pthread_mutex_t));
>> >>                 if (!avs_mutex)
>> >>                         goto init_fail;
>> >>                 if (pthread_mutex_init(avs_mutex, NULL))
>> >>                         goto init_fail;
>> >>         }
>> >>
>> >>         if (pthread_mutex_lock(avs_mutex))
>> >>                 goto init_fail;
>> >>         if (avisynth_load_library())
>> >>                 goto init_fail;
>> >
>> > please dont goto to a single return statement, this obfuscates the
>> > code. And it also forgets unlocking the mutex in this case
>> I will replace goto --> return statements. Regarding the mutex though,
>> trying to unlock a mutex after a constructor failure or similar is an
>> undefined behavior unless I add a bunch more code to handle the return
>> value. I would rather just return a hard error if such a thing is
>> possible in avformat.
>
> maybe iam too tired but i dont understand what undefined behavior you
> refre to here. Not unlocking the mutex though will deadlock you
> the next time the code is run.
>
> also i see no code to destroy the mutex ...
>
If avpriv_* are really the answer, then I suppose this will be a moot
point. I just noticed that the goto after avisynth_library_load
doesn't unlock the mutex, which I didn't see before. Destroying the
mutex safely is a bit trickier than creating it, unfortunately...


I have attached a new version of avisynth.c to this email that uses
avpriv_lock_avformat and avpriv_unlock_avformat.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avisynth.c
Type: text/x-csrc
Size: 14155 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121002/b9afeb6d/attachment.bin>