[vlc-devel] [PATCH] access module for BlackMagic SDI cards
jb at videolan.org
Mon Sep 27 14:32:02 CEST 2010
On Mon, Sep 27, 2010 at 01:56:01PM +0200, Steinar H. Gunderson wrote :
> In any case, I guess this would break the Windows build currently (if the
> user has the SDK installed), so I don't think it's an unreasonable default.
Very unlikely case.
> >> +#ifndef INT64_C
> >> +#define INT64_C(c) c ## LL
> >> +#endif
> > Is this necessary ?
> Yes, it is, given that the module is C++ and the INT64_C stuff is C99.
> >> + /* Note: AddRef() and Release() here are not thread safe. */
> > This is an issue, no?
> Actually I don't think so. The only point where they are AddRef()-ed is from
> Open(), and the only point where they are Release()-ed is in Close() (after
> we've shut down the SDI worker threads). Granted, you could say that the SDK
> might AddRef() and Release() the delegate, but BlackMagic's own example code
> actually just has "return 1;" everywhere here, so I don't think it's a big
> problem in practice.
> Does VLC have incremental inc and dec functions I could use?
vlc_gc_incref and such? Not sure if that applied here.
> >> + int i_card_index = var_CreateGetInteger( p_demux, "sdi-card-index" );
> > Shouldn't you use inherit here?
> Please explain (or point to an example). I have no idea how this stuff works :-)
Ivoire will comment more.
> strncpy() will add a null byte, which is not what we want. I could always use
No, discard my comment.
> >> + p_display_mode->Release();
> >> + p_display_iterator->Release();
> >> + Close( p_this );
> >> + return VLC_EGENERIC;
> > Can't you factor a bit thise code? like a goto error ?
> I wondered what to do about this one, and it's really easy to miss one or two
> releases in some path. I wonder if the cleanest thing is to put _everything_
> in Close(), even though, say, p_display_iterator is never used outside of
> Open(). What would you prefer -- a goto or putting stuff in Close()?
I don't care. Choose what is better, but don't copy-paste error pathes
> >> + msg_Err( p_demux, "Failed to start streams" );
> > What does this mean?
> It means the card refused to start streaming for some reason.
> The documentation lists the following return codes:
> E_FAIL Failure
> S_OK Success
> E_INVALIDARG Is returned on invalid mode or video flags
> E_ACCESSDENIED Unable to access the hardware or input stream currently
> E_OUTOFMEMORY Unable to create a new frame
> Do you want me to reword the message?
> Updated patch below.
Formatted git patch, please.
+33 672 704 734
More information about the vlc-devel