[vlc-devel] [GCI] last.fm access module

[Jakub Wieczorek]

Hello,

2011/1/9 Rémi Denis-Courmont <remi at remlab.net>:
>> Yes, that's intentional. To protect this correctly I'd have to do a
>> mutex lock/unlock on each Read() invocation, which may harm
>> performance, to the best of my knowledge.
>
> You have to protect it somehow. If you really fear the performance cost, use
> an VLC atomic variable. But I am not convinced you will ever notice the
> difference.
>
>> And I'm also under the impression a boolean datum doesn't need to
>> be protected in certain cases such as this one.
>
> There are several problems with atomicity:
>
> - Different architecture support different sizes. There is no warranty that
> sizeof(bool) and signedness of bool matches the CPU architecture constraints.
>
> - At the very least you need a read memory barrier to synchronize the memory
> (e.g. stall the per-CPU cache) with other CPU cores; you have no warranty that
> both threads run on the same CPU.
>
> - Compiler optimizations can do really unexpected things. For instance the
> machine code often keeps values cached in a register if the optimizing
> compiler notices that the value cannot be modified in a code path.
>
>> Please let me know if I'm mistaken.
>>
>> >  - the mutex seems leaked on cancellation
>>
>> Right. I guess that's what mutex_cleanup_push is for? I'll take a look.
>> Thanks.
>
> Yes. When a thread is cancelled at vlc_cond_wait(), the mutex is locked before
> cancellation is acted on. We don't leverage this "feature" ever in VLC, but
> it's the POSIX convention.

Thanks very much. Attaching a revised patch.

Best regards,
Jakub Wieczorek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-an-access-module-that-works-with-last.fm-radio-s.patch
Type: text/x-patch
Size: 42852 bytes
Desc: not available
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20110109/a0672f7d/attachment.bin>