Hi Jean-Baptiste Kempf.<br><br><blockquote>> # if __WORDSIZE == 64<br>
> # define INT64_C(c) c ## L<br>
> # else<br>
> # define INT64_C(c) c ## LL<br>
> # endif<br><br>
This shouldn't be here.<br><br>
> #define CLOCK_FREQ INT64_C(1000000)<br><br>
Already defined in the VLC headers<br></blockquote>
<br>If I comment the define, I have error:<br><br>decklinkoutput.cpp:888: error: ‘INT64_C’ was not declared in this scope.<br><br><blockquote style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;" class="gmail_quote">
I cannot test, but can tell a few remarks, before inclusion:<br>
- no tabs in source code, please,<br>
- cleanup your includes (do you need all of those?) and your defines,<br>
- be careful about your code sytle consistency,<br>
- align code when it helps read it.<br></blockquote><br>Oh, I'm using Eclipse tab. I fixed it.<br><br><br><div class="gmail_quote">On Tue, Aug 16, 2011 at 2:40 PM, Jean-Baptiste Kempf <span dir="ltr"><<a href="mailto:jb@videolan.org">jb@videolan.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin: 0pt 0pt 0pt 0.8ex; border-left: 1px solid rgb(204, 204, 204); padding-left: 1ex;">On Tue, Aug 16, 2011 at 09:29:00AM +0700, Ocon 8866 wrote :<br>
<div class="im">> Please help test my plugin: Decklink output.<br>
<br>
</div>Nice work!<br>
<br>
I cannot test, but can tell a few remarks, before inclusion:<br>
- no tabs in source code, please,<br>
- cleanup your includes (do you need all of those?) and your defines,<br>
- be careful about your code sytle consistency,<br>
- align code when it helps read it.<br>
<br>
> # if __WORDSIZE == 64<br>
> # define INT64_C(c) c ## L<br>
> # else<br>
> # define INT64_C(c) c ## LL<br>
> # endif<br>
<br>
This shouldn't be here.<br>
<br>
> #define CLOCK_FREQ INT64_C(1000000)<br>
<br>
Already defined in the VLC headers.<br>
<br>
> #define DECKLINK_VIDEO_CODEC VLC_CODEC_UYVY<br>
This is the recommended one by Decklink?<br>
<br>
> #define DECKLINK_AUDIO_CODEC VLC_CODEC_S16N<br>
Why not FL32?<br>
<br>
> typedef struct {<br>
><br>
> IDeckLink *p_card;<br>
> IDeckLinkOutput *p_output;<br>
> IDeckLinkConfiguration *p_config;<br>
> IDeckLinkDisplayModeIterator *p_display_iterator;<br>
> IDeckLinkIterator *decklink_iterator;<br>
> int i_card_index;<br>
> char *psz_video_connection;<br>
> BMDDisplayMode display_mode;<br>
> BMDVideoOutputFlags videoOutputFlags;<br>
> char *psz_audio_connection;<br>
> int i_channels;<br>
> int i_rate;<br>
> uint32_t i_dominance_flags;<br>
> int i_width;<br>
> int i_height;<br>
> BMDTimeScale i_timescale;<br>
> BMDTimeValue i_frameduration;<br>
> vlc_mutex_t m_lock;<br>
> vlc_atomic_t m_ref;<br>
> } decklink_sys_t;<br>
<br>
Please do so alignment<br>
<br>
> /*****************************************************************************<br>
> * Module descriptor<br>
> *****************************************************************************/<br>
> //<br>
> //<br>
> vlc_module_begin()<br>
> set_shortname( N_("DecklinkOutput") )<br>
> set_description( N_("Decklink Output plug-in") )<br>
> // add_shortcut( "decklink-output" )<br>
> set_section( N_("Decklink General Options"), NULL )<br>
> add_integer( CFG_PREFIX "card-index", 0,<br>
> CARD_INDEX_TEXT, CARD_INDEX_LONGTEXT, true )<br>
><br>
> add_submodule ()<br>
> // set_shortname (N_("DecklinkVOut"))<br>
> set_description (N_("Decklink Video Output module"))<br>
> set_category(CAT_VIDEO)<br>
> set_subcategory(SUBCAT_VIDEO_VOUT)<br>
> set_capability("vout display", 0)<br>
> set_callbacks (OpenVideo, CloseVideo)<br>
> set_section( N_("Decklink Video Options"), NULL )<br>
> // add_shortcut( "decklink-vout" )<br>
> add_string( VIDEO_CFG_PREFIX "video-connection", 0,<br>
> VIDEO_CONNECTION_TEXT, VIDEO_CONNECTION_LONGTEXT, true )<br>
> change_string_list( ppsz_videoconns, ppsz_videoconns_text, 0 )<br>
> add_string( VIDEO_CFG_PREFIX "mode", "pal ",<br>
> MODE_TEXT, MODE_LONGTEXT, true )<br>
><br>
> add_submodule ()<br>
> // set_shortname (N_("DecklinkAOut"))<br>
> set_description (N_("Decklink Audio Output module"))<br>
> set_category( CAT_AUDIO )<br>
> set_subcategory( SUBCAT_AUDIO_AOUT )<br>
> set_capability( "audio output", 0 )<br>
> set_callbacks (OpenAudio, CloseAudio)<br>
> // add_shortcut( "decklink-aout" )<br>
> set_section( N_("Decklink Audio Options"), NULL )<br>
> add_string( AUDIO_CFG_PREFIX "audio-connection", 0,<br>
> AUDIO_CONNECTION_TEXT, AUDIO_CONNECTION_LONGTEXT, true )<br>
> change_string_list( ppsz_audioconns, ppsz_audioconns_text, 0 )<br>
> add_integer( AUDIO_CFG_PREFIX "audio-rate", 48000,<br>
> RATE_TEXT, RATE_LONGTEXT, true )<br>
> add_integer( AUDIO_CFG_PREFIX "audio-channels", 2,<br>
> CHANNELS_TEXT, CHANNELS_LONGTEXT, true )<br>
> vlc_module_end ()<br>
<br>
You shouldn't need 2 submodules, but only 1 module and 1 submodule.<br>
<br>
> /*****************************************************************************<br>
> * Open: initialize interface , public IDeckLinkAudioOutputCallback<br>
> *****************************************************************************/<br>
><br>
> static int OpenDecklink( vlc_object_t *p_this )<br>
> {<br>
><br>
><br>
> if(!(vlc_atomic_get( &decklink_sys.m_ref ))){<br>
> //if(!(decklink_sys.m_ref)){<br>
> vlc_mutex_init(&decklink_sys.m_lock);<br>
> vlc_mutex_lock(&decklink_sys.m_lock);<br>
> vlc_atomic_set( &(decklink_sys.m_ref), 0 );<br>
><br>
> int i_rate;<br>
> int i_card_index;<br>
> int i_channels;<br>
> BMDVideoOutputFlags videoOutputFlags = 0;<br>
> bool b_found_mode;<br>
><br>
> i_card_index = var_InheritInteger( p_this, CFG_PREFIX "card-index" );<br>
><br>
> //IDeckLinkDisplayModeIterator *p_display_iterator = NULL;<br>
><br>
> decklink_sys.decklink_iterator = CreateDeckLinkIteratorInstance();<br>
> if( !decklink_sys.decklink_iterator )<br>
> {<br>
> msg_Err( p_this, "DeckLink drivers not found." );<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
> }<br>
> HRESULT result;<br>
> if( i_card_index < 0 )<br>
> {<br>
> msg_Err( p_this, "Invalid card index %d", i_card_index );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
> }<br>
> for( int i = 0; i <= i_card_index; ++i )<br>
> {<br>
> if( decklink_sys.p_card ){<br>
><br>
> //decklink_sys.p_card->Release();<br>
> msg_Dbg( p_this, "Error here %d Cannot Release Pcard dc", i);<br>
> }<br>
> result = decklink_sys.decklink_iterator->Next( &decklink_sys.p_card );<br>
> if( result != S_OK )<br>
> break;<br>
> }<br>
> if( result != S_OK )<br>
> {<br>
> msg_Err( p_this, "DeckLink PCI card %d not found", i_card_index );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
> }<br>
><br>
> const char *psz_model_name;<br>
> result = decklink_sys.p_card->GetModelName( &psz_model_name );<br>
><br>
> if( result != S_OK )<br>
> {<br>
> msg_Err( p_this, "Could not get model name" );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
> }<br>
> msg_Dbg( p_this, "Opened DeckLink PCI card %d (%s)", i_card_index, psz_model_name );<br>
> if( decklink_sys.p_card->QueryInterface( IID_IDeckLinkOutput, (void**)&decklink_sys.p_output) != S_OK )<br>
> {<br>
> msg_Err( p_this, "Card has no outputs" );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
> }<br>
> if( decklink_sys.p_card->QueryInterface( IID_IDeckLinkConfiguration, (void**)&decklink_sys.p_config) != S_OK )<br>
> {<br>
> msg_Err( p_this, "Failed to get configuration interface" );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
> }<br>
><br>
> decklink_sys.i_card_index = i_card_index ;<br>
><br>
> //Setting up Video Connection<br>
> char* psz_video_connection;<br>
> psz_video_connection = var_InheritString( p_this, VIDEO_CFG_PREFIX "video-connection" );<br>
> if( psz_video_connection )<br>
> {<br>
> BMDVideoConnection conn;<br>
> if ( !strcmp( psz_video_connection, "sdi" ) )<br>
> conn = bmdVideoConnectionSDI;<br>
> else if ( !strcmp( psz_video_connection, "hdmi" ) )<br>
> conn = bmdVideoConnectionHDMI;<br>
> else if ( !strcmp( psz_video_connection, "opticalsdi" ) )<br>
> conn = bmdVideoConnectionOpticalSDI;<br>
> else if ( !strcmp( psz_video_connection, "component" ) )<br>
> conn = bmdVideoConnectionComponent;<br>
> else if ( !strcmp( psz_video_connection, "composite" ) )<br>
> conn = bmdVideoConnectionComposite;<br>
> else if ( !strcmp( psz_video_connection, "svideo" ) )<br>
> conn = bmdVideoConnectionSVideo;<br>
> else<br>
> {<br>
> msg_Err( p_this, "Invalid video-connection specified; choose one of " \<br>
> "sdi, hdmi, opticalsdi, component, composite, or svideo." );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> free( psz_video_connection );<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
<br>
leaking psz_video_connection;<br>
You probably need to do some common error path.<br>
<br>
> }<br>
> msg_Dbg( p_this, "Setting video output connection to 0x%x", conn);<br>
> result = decklink_sys.p_config->SetInt( bmdDeckLinkConfigVideoOutputConnection, conn );<br>
> if( result != S_OK )<br>
> {<br>
> msg_Err( p_this, "Failed to set video output connection" );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> free( psz_video_connection );<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
<br>
idem<br>
<br>
> }<br>
><br>
> decklink_sys.psz_video_connection = psz_video_connection;<br>
> free( psz_video_connection );<br>
> }<br>
><br>
> //End of Setting up Video Connection<br>
><br>
><br>
> // Setting up Audio Connection<br>
> char *psz_audio_connection;<br>
> psz_audio_connection = var_CreateGetNonEmptyString( p_this, AUDIO_CFG_PREFIX "audio-connection" );<br>
> if( psz_audio_connection )<br>
> {<br>
> BMDAudioConnection conn;<br>
> if ( !strcmp( psz_audio_connection, "embedded" ) )<br>
> conn = bmdAudioConnectionEmbedded;<br>
> else if ( !strcmp( psz_audio_connection, "aesebu" ) )<br>
> conn = bmdAudioConnectionAESEBU;<br>
> else if ( !strcmp( psz_audio_connection, "analog" ) )<br>
> conn = bmdAudioConnectionAnalog;<br>
> else<br>
> {<br>
> msg_Err( p_this, "Invalid audio-connection specified; choose one of " \<br>
> "embedded, aesebu, or analog." );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> free( psz_audio_connection );<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
<br>
idem for psz_audio_connection<br>
<br>
> }<br>
><br>
> msg_Dbg( p_this, "Setting audio output format to 0x%x", conn);<br>
> result = decklink_sys.p_config->SetInt( bmdDeckLinkConfigAudioInputConnection, conn );<br>
> if( result != S_OK )<br>
> {<br>
> msg_Err( p_this, "Failed to set audio output connection" );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> free( psz_audio_connection );<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
> }<br>
><br>
> decklink_sys.psz_audio_connection = psz_audio_connection;<br>
> free( psz_audio_connection );<br>
> }<br>
> // End of Setting up Audio Connection<br>
><br>
> //Find Display Mode<br>
><br>
> result = decklink_sys.p_output->GetDisplayModeIterator( &decklink_sys.p_display_iterator );<br>
> if( result != S_OK )<br>
> {<br>
> msg_Err( p_this, "Failed to enumerate display modes" );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
> }<br>
><br>
> char *psz_display_mode;<br>
> psz_display_mode = var_CreateGetNonEmptyString( p_this, VIDEO_CFG_PREFIX "mode" );<br>
> if( !psz_display_mode || strlen( psz_display_mode ) > 4 ) {<br>
> msg_Err( p_this, "Missing or invalid mode string" );<br>
> if( decklink_sys.decklink_iterator )<br>
> decklink_sys.decklink_iterator->Release();<br>
> if( decklink_sys.p_display_iterator )<br>
> decklink_sys.p_display_iterator->Release();<br>
> vlc_mutex_unlock(&decklink_sys.m_lock);<br>
> return VLC_EGENERIC;<br>
> }<br>
><br>
> /*<br>
> * Pad the --decklink-mode string to four characters, so the user can specify e.g. "pal"<br>
> * without having to add the trailing space.<br>
> */<br>
> char sz_display_mode_padded[5];<br>
> strcpy(sz_display_mode_padded, " ");<br>
<br>
Ugly init<br>
<br>
> for( int i = 0; i < strlen( psz_display_mode ); ++i )<br>
> sz_display_mode_padded[i] = psz_display_mode[i];<br>
<br>
<br>
<br>
Best Regards,<br>
<font color="#888888"><br>
--<br>
Jean-Baptiste Kempf<br>
<a href="http://www.jbkempf.com/" target="_blank">http://www.jbkempf.com/</a> - +33 672 704 734<br>
Sent from my Electronic Device<br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="http://mailman.videolan.org/listinfo/vlc-devel" target="_blank">http://mailman.videolan.org/listinfo/vlc-devel</a><br>
</font></blockquote></div><br>