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

[d s]

On Tue, Oct 2, 2012 at 7:51 PM, d s <avxsynth.testing at gmail.com> wrote:
> 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.

This revision addresses some more concerns by michaelni and fixes the
error that prevented all audio packets from being read. I will make
the comments more complete and doxygen-compatible in a later revision.

However, I've noticed that I'm getting segfaults on exit in ffmpeg-git
(1.0 and 0.11 are fine). I must be missing something, because gdb's
backtrace shows Avisynth being closed before avisynth_read_close() is
reached, leading to a double-free.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: avisynth.c
Type: text/x-csrc
Size: 15697 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121008/d102b082/attachment.bin>