[vlc-devel] [PATCH] Decklink output

[Rémi Denis-Courmont]

Le mardi 22 janvier 2013 18:42:22, Rafaël Carré a écrit :
> >> +#define VIDEO_CONNECTION_TEXT N_("Video connection")
> >> +#define VIDEO_CONNECTION_LONGTEXT N_(\
> >> +    "Video connection to use for DeckLink output. " \
> >> +    "Valid choices: sdi, hdmi, opticalsdi, component, " \
> >> +    "composite, svideo. " \
> >> +    "Leave blank for card default.")
> > 
> > That won't work in typical preferences UI.
> > You should have a default choice in the list instead.
> 
> The list is already here, I removed the choices from help text and chose
> a sane default

I meant that the default value ought to be in the choices list, otherwise the 
corresponding Ui widget will not be built correctly.

And yes, the list of choices should be removed from long text too.

(...)
> >> +    vlc_mutex_unlock(&video_lock);
> > 
> > There is no warranty that close and open run on the same thread. Well
> > maybe the current video output works that way right now, but that's not a
> > very safe and future-proof assumption. Thus a semaphore would be more
> > appropriate than a mutex.
> 
> First I am not sure how semaphores can be used here?
> We don't want to block if decklink module is already in use (e.g.
> equivalent to trylock), also I see no static constructor.

vlc_sem_trywait() can be added if it is required. There is no static 
constructor because POSIX does not define one. But given this plugin already 
has destructor function, adding a constructor function to initialize the 
semaphore would not be much of an extra (portability) problem.

However, on second thoughts, if you never sleep on the synchronization object, 
you should use an atomic variable, not a semaphore.

> Also I only added those mutexes today, to prevent e.g. libvlc users to
> load 2 libvlc instances in the same process, so I could remove them and
> ignore this corner case.
> One libvlc instance will work because we can only have one vout and one
> aout.
> 
> static global data (decklink_sys) is used to synchronize audio and video
> output modules atm.
> If there is a way to allocate it dynamically at first use and share the
> pointer between the 2 modules, we could get rid of these mutexes
> (video_lock and audio_lock).

The cleanest way I can think of would consist of a data structure mapping card 
indexes to the shared state structure for each card. That would be protected 
by a static mutex.

That would still need a reference counter or a destructor to clean up.

> And we could have 2 libvlc instances in one process, talking to 2
> different cards.
> 
> I am not sure how config_Get helps that?

The point is just that if the configuration is shared across the process, it 
should be fetched with config_Get() rather than var_Get() or var_Inherit().

> I was looking for code using var_InheritPointer but this function
> doesn't exist .. any hint?

-- 
Rémi Denis-Courmont
http://www.remlab.net/