<div dir="ltr"><div><div>Remi,<br><br></div><div>Just wanted to add. <br></div>Semaphore usage can be avoided without causing any frame drops. I'll remove that code.<br><br></div>Thanks,<br>Vikram<br><div><div class="gmail_extra">
<br><br><div class="gmail_quote">On Mon, Apr 21, 2014 at 11:03 PM, ved kpl <span dir="ltr"><<a href="mailto:ved.kpl@gmail.com" target="_blank">ved.kpl@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr">Thanks Remi. Comments inline.<br><div class="gmail_extra"><br><br><div class="gmail_quote"><div><div class="h5">On Mon, Apr 21, 2014 at 9:15 PM, Rémi Denis-Courmont <span dir="ltr"><<a href="mailto:remi@remlab.net" target="_blank">remi@remlab.net</a>></span> wrote:<br>

</div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5">Le samedi 19 avril 2014, 13:51:56 Vikram Fugro a écrit :<br>
<div>> This module currently supports h264, mpeg4,<br>
> vp8, mpeg2, flashvideo, wmv1/2/3, vc1 decoder<br>
> plugins provided by GStreamer 1.0 library.<br>
><br>
> Signed-off-by: Vikram Fugro <<a href="mailto:ved.kpl@gmail.com" target="_blank">ved.kpl@gmail.com</a>><br>
> ---<br>
>  <a href="http://configure.ac" target="_blank">configure.ac</a>              |    5 +<br>
>  modules/codec/Makefile.am |    7 +<br>
>  modules/codec/gstcodec.c  |  998<br>
> +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1010<br>
> insertions(+)<br>
>  create mode 100644 modules/codec/gstcodec.c<br>
><br>
> diff --git a/<a href="http://configure.ac" target="_blank">configure.ac</a> b/<a href="http://configure.ac" target="_blank">configure.ac</a><br>
> index 2f28385..5dc23e4 100644<br>
> --- a/<a href="http://configure.ac" target="_blank">configure.ac</a><br>
> +++ b/<a href="http://configure.ac" target="_blank">configure.ac</a><br>
> @@ -2707,6 +2707,11 @@ dnl<br>
>  PKG_ENABLE_MODULES_VLC([THEORA], [], [ogg theoradec >= 1.0 theoraenc],<br>
> [experimental theora codec], [auto])<br>
><br>
>  dnl<br>
> +dnl GStreamer plugins<br>
> +dnl<br>
> +PKG_ENABLE_MODULES_VLC([GSTREAMER], [gstcodec], [gstreamer-1.0<br>
> gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-basevideo-1.0<br>
> gstreamer-video-1.0], [gstreamer], [auto])<br>
<br>
</div>I can't find some of those anywhere. Also I am not sure merging generic and<br>
video libraries in a single variable is a good idea.<br>
</div></div><div><div>Vikram: Which ones? You suggest to have more than one PKG_ENABLE_MODULES? <br></div></div></blockquote><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5">
<div><div>
> +<br>
> +dnl<br>
>  dnl  schroedinger decoder plugin (for dirac format video)<br>
>  dnl<br>
>  PKG_ENABLE_MODULES_VLC([SCHROEDINGER], [], [schroedinger-1.0 >= 1.0.10],<br>
> [dirac decoder and encoder using schroedinger], [auto])<br>
> diff --git<br>
> a/modules/codec/Makefile.am b/modules/codec/Makefile.am index<br>
> 14a53fd..0c0de5e 100644<br>
> --- a/modules/codec/Makefile.am<br>
> +++ b/modules/codec/Makefile.am<br>
> @@ -464,3 +464,10 @@ libquicktime_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath<br>
> '$(codecdir)' libquicktime_plugin_la_LIBADD = $(LIBM)<br>
>  EXTRA_LTLIBRARIES += <a href="http://libquicktime_plugin.la" target="_blank">libquicktime_plugin.la</a><br>
>  codec_LTLIBRARIES += $(LTLIBquicktime)<br>
> +<br>
> +libgstcodec_plugin_la_SOURCES = codec/gstcodec.c<br>
> +libgstcodec_plugin_la_CFLAGS = $(AM_CFLAGS) $(GSTREAMER_CFLAGS)<br>
> +libgstcodec_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(codecdir)'<br>
> +libgstcodec_plugin_la_LIBADD = $(GSTREAMER_LIBS)<br>
> +EXTRA_LTLIBRARIES += <a href="http://libgstcodec_plugin.la" target="_blank">libgstcodec_plugin.la</a><br>
> +codec_LTLIBRARIES += $(LTLIBgstcodec)<br>
> diff --git a/modules/codec/gstcodec.c b/modules/codec/gstcodec.c<br>
> new file mode 100644<br>
> index 0000000..43319a7<br>
> --- /dev/null<br>
> +++ b/modules/codec/gstcodec.c<br>
> @@ -0,0 +1,998 @@<br>
> +/**************************************************************************<br>
</div></div>> *** + * gstcodec.c: video decoder module making use of gstreamer<br>
> +<br>
> ***************************************************************************<br>
> ** + * Copyright (C) 1999-2014 the VideoLAN team<br>
<br>
1999?<br>
</div></div><div>Vikram: Yeah, my mistake! it should just be 2014, right?<br></div></blockquote><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div class="h5">
<div>
> + * $Id:<br>
> + *<br>
> + * Author: Vikram Fugro <<a href="mailto:ved.kpl@gmail.com" target="_blank">ved.kpl@gmail.com</a>><br>
> + *<br>
> + * This program is free software; you can redistribute it and/or modify<br>
> + * it under the terms of the GNU General Public License as published by<br>
> + * the Free Software Foundation; either version 2 of the License, or<br>
> + * (at your option) any later version.<br>
> + *<br>
> + * This program is distributed in the hope that it will be useful,<br>
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of<br>
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the<br>
> + * GNU General Public License for more details.<br>
> + *<br>
> + * You should have received a copy of the GNU General Public License<br>
> + * along with this program; if not, write to the Free Software<br>
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA 02110-1301,<br>
> USA. +<br>
> ***************************************************************************<br>
> **/ +<br>
> +/**************************************************************************<br>
> *** + * Preamble<br>
> +<br>
> ***************************************************************************<br>
</div>> **/ +#ifdef HAVE_CONFIG_H<br>
<div><div>> +# include "config.h"<br>
> +#endif<br>
> +<br>
> +#include <vlc_common.h><br>
> +#include <vlc_plugin.h><br>
> +#include <vlc_codec.h><br>
> +#include <vlc_sout.h><br>
> +#include <vlc_input.h><br>
> +<br>
> +#include <assert.h><br>
> +#include <limits.h><br>
> +<br>
> +#include <gst/gst.h><br>
> +#include <gst/video/gstvideometa.h><br>
> +#include <gst/video/video.h><br>
> +#include <gst/app/gstappsrc.h><br>
> +#include <gst/gstatomicqueue.h><br>
> +<br>
> +struct decoder_sys_t<br>
> +{<br>
> +    GstElement* p_decoder;<br>
> +    GstElement* p_decode_src;<br>
> +    GstElement* p_decode_in;<br>
> +    GstElement* p_decode_out;<br>
> +<br>
> +    GstBus* p_bus;<br>
> +<br>
> +    GstVideoInfo vinfo;<br>
> +    GstAtomicQueue *p_que;<br>
> +    bool b_prerolled;<br>
> +    bool b_running;<br>
> +<br>
> +    bool b_new_segment_pending;<br>
> +    bool b_out_fmt_set;<br>
> +    vlc_sem_t sem_mt;<br>
> +};<br>
> +<br>
> +/**************************************************************************<br>
</div></div>> *** + * Local prototypes<br>
> +<br>
> ***************************************************************************<br>
> **/ +static int  OpenDecoder( vlc_object_t * );<br>
<div>> +static void CloseDecoder( vlc_object_t * );<br>
> +static picture_t *DecodeBlock( decoder_t *, block_t ** );<br>
> +<br>
> +#define MODULE_DESCRIPTION N_( "Various video decoders " \<br>
> +        "delivered by the GStreamer library.")<br>
> +<br>
> +#define USEDECODEBIN_TEXT N_("Use DecodeBin")<br>
> +#define USEDECODEBIN_LONGTEXT N_( \<br>
> +    "Let DecodeBin choose the decoder." )<br>
<br>
</div>So hmm, why should the user have the choice and on which basis should (s)he<br>
make that choice?<br>
<br>
In my limited understanding of gstreamer, decodebin provides demux, packetizer<br>
and decoder together as one wrapper element. Is there any benefit to decodebin<br>
for a VLC decoder plugin? Conversely, is there any disadvantage to calling the<br>
specific gstreamer decoder elements directly?<br>
</div></div><div><div>Vikram: There is not really a disadvantage. Just that decodebin adds video parsers also (besides decoders)  that provides more info (GstCaps-wise) to the decoder.<br></div></div></blockquote><div>                If you think it is not really needed, I 'll have it removed. <br>

</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>
> +<br>
> +vlc_module_begin ()<br>
> +    set_shortname( "GStreamer")<br>
> +    add_shortcut( "gstreamer" )<br>
> +    set_category( CAT_INPUT )<br>
> +    set_subcategory( SUBCAT_INPUT_VCODEC )<br>
> +    /* decoder main module */<br>
> +    set_description( N_( "GStreamer video decoder" ) )<br>
> +    set_help( MODULE_DESCRIPTION )<br>
> +    set_capability( "decoder", 50 )<br>
> +    set_section( N_( "Decoding" ) , NULL )<br>
> +    set_callbacks( OpenDecoder, CloseDecoder )<br>
> +    add_bool( "use-decodebin", false, USEDECODEBIN_TEXT,<br>
> +        USEDECODEBIN_LONGTEXT, false)<br>
> +vlc_module_end ()<br>
> +<br>
> +static vlc_fourcc_t gst_to_vlc_fmt( const gchar* psz_fmt )<br>
> +{<br>
> +    if ( !psz_fmt )<br>
> +        return 0;<br>
> +<br>
> +    if( !strcmp ( psz_fmt, "I420" ) )<br>
> +        return VLC_CODEC_I420;<br>
> +    if( !strcmp ( psz_fmt, "YV12" ) )<br>
> +        return VLC_CODEC_YV12;<br>
> +    if( !strcmp ( psz_fmt, "YUY2" ) ||<br>
> +            !strcmp ( psz_fmt, "YUYV" ) )<br>
> +        return VLC_CODEC_YUYV;<br>
> +    if( !strcmp ( psz_fmt, "UYVY" ) )<br>
> +        return VLC_CODEC_UYVY;<br>
> +    if( !strcmp ( psz_fmt, "VYUY" ) )<br>
> +        return VLC_CODEC_VYUY;<br>
> +    if( !strcmp ( psz_fmt, "NV12" ) )<br>
> +        return VLC_CODEC_NV12;<br>
> +    if( !strcmp ( psz_fmt, "NV21" ) )<br>
> +        return VLC_CODEC_NV21;<br>
> +<br>
> +    return 0;<br>
> +}<br>
> +<br>
> +static GstStructure* vlc_to_gst_fmt( vlc_fourcc_t i_fmt )<br>
> +{<br>
> +    GstStructure* p_str = NULL;<br>
> +<br>
> +    switch( i_fmt )<br>
> +    {<br>
> +        case VLC_CODEC_H264:<br>
> +            p_str = gst_structure_new_empty( "video/x-h264" );<br>
> +            gst_structure_set( p_str, "alignment", G_TYPE_STRING, "au",<br>
> NULL ); +            break;<br>
> +        case VLC_CODEC_MP4V:<br>
> +            p_str = gst_structure_new_empty( "video/mpeg" );<br>
> +            gst_structure_set( p_str, "mpegversion", G_TYPE_INT, 4,<br>
> +                    "systemstream", G_TYPE_BOOLEAN, FALSE, NULL );<br>
> +            break;<br>
> +        case VLC_CODEC_VP8:<br>
> +            p_str = gst_structure_new_empty( "video/x-vp8" );<br>
> +            break;<br>
> +        case VLC_CODEC_MPGV:<br>
> +            p_str = gst_structure_new_empty( "video/mpeg" );<br>
> +            gst_structure_set( p_str, "mpegversion", G_TYPE_INT, 2,<br>
> +                    "systemstream", G_TYPE_BOOLEAN, FALSE, NULL );<br>
> +            break;<br>
> +        case VLC_CODEC_FLV1:<br>
> +            p_str = gst_structure_new_empty( "video/x-flash-video" );<br>
> +            gst_structure_set( p_str, "flvversion", G_TYPE_INT, 1, NULL );<br>
> +            break;<br>
> +        case VLC_CODEC_WMV1:<br>
> +            p_str = gst_structure_new_empty( "video/x-wmv" );<br>
> +            gst_structure_set( p_str, "wmvversion", G_TYPE_INT, 1,<br>
> +                "format", G_TYPE_STRING, "WMV1", NULL );<br>
> +            break;<br>
> +        case VLC_CODEC_WMV2:<br>
> +            p_str = gst_structure_new_empty( "video/x-wmv" );<br>
> +            gst_structure_set( p_str, "wmvversion", G_TYPE_INT, 2,<br>
> +                "format", G_TYPE_STRING, "WMV2", NULL );<br>
> +            break;<br>
> +        case VLC_CODEC_WMV3:<br>
> +            p_str = gst_structure_new_empty( "video/x-wmv" );<br>
> +            gst_structure_set( p_str, "wmvversion", G_TYPE_INT, 3,<br>
> +                "format", G_TYPE_STRING, "WMV3", NULL );<br>
> +            break;<br>
> +        case VLC_CODEC_VC1:<br>
> +            p_str = gst_structure_new_empty( "video/x-wmv" );<br>
> +            gst_structure_set( p_str, "wmvversion", G_TYPE_INT, 3,<br>
> +                "format", G_TYPE_STRING, "WVC1", NULL );<br>
> +            break;<br>
> +        default:<br>
> +            break;<br>
> +    }<br>
> +<br>
> +    return p_str;<br>
> +}<br>
<br>
</div></div>You should probably take a constant pointer to the whole es_format_t and set<br>
the other parameters - or create an intermediate function around this one.<br></blockquote></div></div><div>   Vikram: Not very much clear to me.. What is "other parameters" here? <br></div><div class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">


<div><br>
> +<br>
> +/* Probe for the buffer just pushed into appsrc<br>
> + * Maintains sync between the DecodeBlock() and<br>
> + * the appsrc */<br>
> +static GstPadProbeReturn buffer_probe_cb( GstPad* p_pad,<br>
> +        GstPadProbeInfo* p_info, gpointer p_data )<br>
> +{<br>
> +    decoder_t* p_dec = ( decoder_t* )p_data;<br>
> +    vlc_sem_post( &p_dec->p_sys->sem_mt );<br>
> +    return GST_PAD_PROBE_OK;<br>
> +}<br>
> +<br>
> +/* Emitted by appsrc when serving a seek request.<br>
> + * Seek over here is only used for flushing the buffers.<br>
> + * Returns TRUE always, as the 'real' seek will be<br>
> + * done by VLC framework */<br>
<br>
</div>I am not sure that makes much sense. I mean, can this event ever happen when<br>
the decoder is between an appsink and an appsrc elements?<br></blockquote></div><div>   Vikram: Yes, it does, The main motive is not seek but to flush the buffers. That is why I send a flushing seek event to the bin.<br>
</div><div><div class="h5">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div><br>
> +static gboolean seek_data_cb( GstAppSrc* p_src, guint64 l_offset,<br>
> +        gpointer p_data )<br>
> +{<br>
> +    decoder_t* p_dec = ( decoder_t* )p_data;<br>
> +    msg_Dbg( p_dec, "appsrc seeking to %llu", (unsigned long long)l_offset<br>
> );<br>
> +    return TRUE;<br>
> +}<br>
> +<br>
> +/* Emitted by decodebin when there are no more<br>
> + * outputs. In this case it will be mostly and<br>
> + * should be one */<br>
> +static void no_more_pads_cb( GstElement* p_ele, gpointer p_data )<br>
> +{<br>
> +    decoder_t* p_dec = ( decoder_t* )p_data;<br>
> +    GstPad* p_pad;<br>
> +<br>
> +    msg_Dbg( p_dec, "no more pads" );<br>
> +<br>
> +    p_pad = gst_element_get_static_pad( p_dec->p_sys->p_decode_out,<br>
> +            "sink" );<br>
> +    if( p_pad == NULL || !gst_pad_is_linked( p_pad ) )<br>
> +    {<br>
> +        gst_element_post_message ( GST_ELEMENT( p_dec->p_sys->p_decoder ),<br>
> +                gst_message_new_application( GST_OBJECT(<br>
> +                        p_dec->p_sys->p_decoder ),<br>
> +                    gst_structure_new( "VlcGstError",<br>
> +                        "message", G_TYPE_STRING,<br>
> +                        "failed to link decode out pad",<br>
> +                        NULL ) ) );<br>
> +    }<br>
> +<br>
> +    gst_object_unref( p_pad );<br>
> +}<br>
> +<br>
> +/* Emitted by decodebin and links decodebin to fakesink*/<br>
> +static void pad_added_cb( GstElement* p_ele, GstPad* p_pad, gpointer p_data<br>
> )<br>
> +{<br>
> +    decoder_t* p_dec = ( decoder_t* )p_data;<br>
> +    GstPad* p_sinkpad;<br>
> +<br>
> +    p_sinkpad = gst_element_get_static_pad(<br>
> +            p_dec->p_sys->p_decode_out, "sink" );<br>
> +<br>
> +    if( !gst_pad_is_linked( p_sinkpad ) )<br>
> +    {<br>
> +        GstCaps* p_caps;<br>
> +<br>
> +        /* Get the output format type */<br>
> +        p_caps = gst_pad_get_current_caps( p_pad );<br>
> +        if( p_caps )<br>
> +        {<br>
> +            GstStructure* p_str;<br>
> +<br>
> +            p_str = gst_caps_get_structure( p_caps, 0 );<br>
> +<br>
> +            /* We want raw caps only */<br>
> +            if( gst_structure_has_name( p_str, "video/x-raw" ) ) {<br>
> +                if( !gst_video_info_from_caps( &p_dec->p_sys->vinfo,<br>
> +                            p_caps ) )<br>
> +                {<br>
> +                    gst_element_post_message(<br>
> +                            GST_ELEMENT( p_dec->p_sys->p_decoder ),<br>
> +                            gst_message_new_application( GST_OBJECT(<br>
> +                                    p_dec->p_sys->p_decoder ),<br>
> +                                gst_structure_new( "VlcGstWarning",<br>
> +                                    "message", G_TYPE_STRING,<br>
> +                                    "failed to get video info from caps",<br>
> +                                    NULL ) ) );<br>
> +                } else<br>
> +                    gst_pad_link( p_pad, p_sinkpad );<br>
> +            }<br>
> +<br>
> +            gst_caps_unref( p_caps );<br>
> +        }<br>
> +    }<br>
> +<br>
> +    gst_object_unref( p_sinkpad );<br>
> +}<br>
> +<br>
> +/* Emitted by fakesink for every buffer and sets the<br>
> + * output format in VLC context for vout. Adds the<br>
> + * buffer to the queue */<br>
> +static void frame_handoff_cb( GstElement* p_ele, GstBuffer* p_buf,<br>
> +        GstPad* p_pad, gpointer p_data )<br>
> +{<br>
> +    decoder_t* p_dec = ( decoder_t* )p_data;<br>
> +<br>
> +    if( unlikely( p_dec->p_sys->b_out_fmt_set == false ) )<br>
> +    {<br>
> +        do<br>
> +        {<br>
> +            if( gst_pad_has_current_caps( p_pad ) )<br>
> +            {<br>
> +                GstCaps* p_caps;<br>
> +<br>
> +                p_caps = gst_pad_get_current_caps( p_pad );<br>
> +                if( p_caps )<br>
> +                {<br>
> +                    GstStructure* p_str;<br>
> +<br>
> +                    p_str = gst_caps_get_structure( p_caps, 0 );<br>
> +                    if( p_str && gst_structure_has_name( p_str,<br>
> "video/x-raw" ) )<br>
> +                    {<br>
> +                        gboolean b_ret = FALSE;<br>
> +<br>
> +                        if( !gst_video_info_from_caps(<br>
> &p_dec->p_sys->vinfo,<br>
> +                                    p_caps ) )<br>
> +                        {<br>
> +                            gst_element_post_message(<br>
> +                                    GST_ELEMENT( p_dec->p_sys->p_decoder ),<br>
> +                                    gst_message_new_application(<br>
> GST_OBJECT( +<br>
> p_dec->p_sys->p_decoder ), +<br>
> gst_structure_new( "VlcGstWarning", +<br>
>      "message", G_TYPE_STRING, +<br>
> "failed to get video info from caps", +<br>
>        NULL ) ) );<br>
> +                            gst_caps_unref( p_caps );<br>
> +                            break;<br>
> +                        }<br>
> +                        else<br>
> +                        {<br>
> +                            p_dec->fmt_out.i_codec = gst_to_vlc_fmt(<br>
> +                                    gst_structure_get_string ( p_str,<br>
> "format" ) ); +<br>
> +                            if( !p_dec->fmt_out.i_codec )<br>
> +                            {<br>
> +                                gst_element_post_message(<br>
> +                                        GST_ELEMENT(<br>
> p_dec->p_sys->p_decoder ), +<br>
> gst_message_new_application( GST_OBJECT( +<br>
>               p_dec->p_sys->p_decoder ), +<br>
>           gst_structure_new( "VlcGstWarning", +<br>
>                    "message", G_TYPE_STRING, +<br>
>                   "invalid output format", +<br>
>                 NULL ) ) );<br>
> +                                gst_caps_unref( p_caps );<br>
> +                                break;<br>
> +                            }<br>
> +<br>
> +                            b_ret = gst_structure_get_fraction( p_str,<br>
> +                                    "pixel-aspect-ratio",<br>
> +                                    ( gint*<br>
> )&p_dec->fmt_out.video.i_sar_num, +                                    (<br>
> gint* )&p_dec->fmt_out.video.i_sar_den ); +<br>
> +                            if( !b_ret || !p_dec->fmt_out.video.i_sar_num<br>
> || +                                    !p_dec->fmt_out.video.i_sar_den ) +<br>
>                            {<br>
> +                                p_dec->fmt_out.video.i_sar_num = 1;<br>
> +                                p_dec->fmt_out.video.i_sar_den = 1;<br>
> +                            }<br>
> +<br>
> +                            b_ret = gst_structure_get_int ( p_str, "width",<br>
> +                                    ( gint* )&p_dec->fmt_out.video.i_width<br>
> ); +                            b_ret &= gst_structure_get_int ( p_str,<br>
> "height", +                                    ( gint*<br>
> )&p_dec->fmt_out.video.i_height ); +<br>
> +                            if( !b_ret || !p_dec->fmt_out.video.i_width ||<br>
> +                                    !p_dec->fmt_out.video.i_height )<br>
> +                            {<br>
> +                                p_dec->fmt_out.video.i_width =<br>
> +                                    p_dec->fmt_in.video.i_width;<br>
> +                                p_dec->fmt_out.video.i_height =<br>
> +                                    p_dec->fmt_in.video.i_height;<br>
> +                            }<br>
> +<br>
> +                            b_ret = gst_structure_get_fraction ( p_str,<br>
> "framerate", +                                    ( gint*<br>
> )&p_dec->fmt_out.video.i_frame_rate, +                                    (<br>
> gint* )&p_dec->fmt_out.video.i_frame_rate_base +<br>
>         );<br>
> +<br>
> +                            if( !b_ret ||<br>
> !p_dec->fmt_out.video.i_frame_rate || +<br>
> !p_dec->fmt_out.video.i_frame_rate_base ) +                            {<br>
> +                                p_dec->fmt_out.video.i_frame_rate =<br>
> +                                    p_dec->fmt_in.video.i_frame_rate;<br>
> +                                p_dec->fmt_out.video.i_frame_rate_base =<br>
> +                                    p_dec->fmt_in.video.i_frame_rate_base;<br>
> +                            }<br>
> +                        }<br>
> +                    }<br>
<br>
</div></div>I think you should split the GstVideoInfo->video_format_t conversion to a<br>
separate function.<br></blockquote></div></div><div>   Vikram: Agreed.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5">

<div><div><br>
> +                    gst_caps_unref( p_caps );<br>
> +                    p_dec->p_sys->b_out_fmt_set = true;<br>
> +                }<br>
> +            }<br>
> +        } while( 0 );<br>
> +<br>
> +        if( !p_dec->p_sys->b_out_fmt_set )<br>
> +            return;<br>
> +    }<br>
> +<br>
> +    /* Push the buffer to the queue */<br>
> +    gst_atomic_queue_push( p_dec->p_sys->p_que, gst_buffer_ref( p_buf ) );<br>
> +}<br>
> +<br>
> +/* Copy the frame data from the GstBuffer (from decoder)<br>
> + * to the picture obtained from downstream in VLC */<br>
> +static void gst_CopyPicture( picture_t *p_pic, GstVideoFrame *frame )<br>
> +{<br>
> +    int i_plane, i_planes, i_line, i_dst_stride, i_src_stride;<br>
> +    uint8_t *p_dst, *p_src;<br>
> +    int i_w, i_h;<br>
> +<br>
> +    i_planes = p_pic->i_planes < 3 ? p_pic->i_planes : 3;<br>
> +    for( i_plane = 0; i_plane < i_planes; i_plane++ )<br>
> +    {<br>
> +        p_dst = p_pic->p[i_plane].p_pixels;<br>
> +        p_src = GST_VIDEO_FRAME_PLANE_DATA( frame, i_plane );<br>
> +        i_dst_stride  = p_pic->p[i_plane].i_pitch;<br>
> +        i_src_stride  = GST_VIDEO_FRAME_PLANE_STRIDE( frame, i_plane );<br>
> +<br>
> +        i_w = GST_VIDEO_FRAME_COMP_WIDTH( frame,<br>
> +                i_plane ) * GST_VIDEO_FRAME_COMP_PSTRIDE( frame, i_plane );<br>
> +        i_h = GST_VIDEO_FRAME_COMP_HEIGHT( frame, i_plane );<br>
> +<br>
> +        for( i_line = 0;<br>
> +                i_line < __MIN( p_pic->p[i_plane].i_lines, i_h );<br>
> +                i_line++ )<br>
> +        {<br>
> +            memcpy( p_dst, p_src, i_w );<br>
> +            p_src += i_src_stride;<br>
> +            p_dst += i_dst_stride;<br>
> +        }<br>
> +    }<br>
> +}<br>
<br>
</div></div>OK in the beginning, but zero-copy is highly desirable here.<br>
</div></div><div>Vikram: Definitely!! That is _THE_ most important thing and will need careful handling.<br></div><div><div class="h5"><div>
<br>
> +<br>
> +/* Check if the element can use this caps */<br>
> +static gint find_decoder_func( gconstpointer p_p1, gconstpointer p_p2 )<br>
> +{<br>
> +    GstElementFactory *p_factory;<br>
> +    GstCaps* p_caps;<br>
> +<br>
> +    p_factory = ( GstElementFactory* )p_p1;<br>
> +    p_caps = ( GstCaps * )p_p2;<br>
> +<br>
> +    return( !gst_element_factory_can_sink_any_caps( p_factory, p_caps ) );<br>
> +}<br>
> +<br>
> +static gint compare_ranks_func( gconstpointer p_p1, gconstpointer p_p2 )<br>
> +{<br>
> +    return gst_plugin_feature_rank_compare_func( p_p1, p_p2 );<br>
> +}<br>
> +<br>
> +/**************************************************************************<br>
</div>> *** + * OpenDecoder: probe the decoder and return score<br>
<div>> +<br>
> ***************************************************************************<br>
> **/<br>
> +static int OpenDecoder( vlc_object_t *p_this )<br>
> +{<br>
> +    decoder_t *p_dec = ( decoder_t* )p_this;<br>
> +    decoder_sys_t *p_sys;<br>
> +    GstStateChangeReturn i_ret;<br>
> +    gboolean b_ret;<br>
> +    GstPad* p_pad;<br>
> +    GstCaps* p_caps = NULL;<br>
> +    GstStructure* p_str = NULL;<br>
> +    GstAppSrcCallbacks cb;<br>
> +    guint8* p_data = NULL;<br>
> +    int i_rval = VLC_SUCCESS;<br>
> +    GList* p_list = NULL;<br>
> +    bool dbin;<br>
> +#define VLC_GST_CHECK(r, v, s, t, x) \<br>
> +    { if(r == v) { msg_Info(p_dec, s); i_rval = t; goto x; } }<br>
<br>
</div>x is a constant here... But IMHO, goto macros are evil.<br>
</div></div><div>Vikram: I agree, but the current usage is limited to this function only.<br></div></blockquote><div>  <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div class="">
<div>
> +<br>
> +    gst_init (NULL, NULL);<br>
<br>
</div>Is this reentrant? If not, you need to serialize.<br>
</div><div>Vikram: Thanks for pointing this out. Yeah, it is not reentrant. Will need a thread-safe wrapper. We will need a common file for gst APIs. In what location should we place this file Should this be built as a different shared library or be part of core vlc libs? <br>

</div></blockquote><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class=""><div>
> +<br>
> +    p_str = vlc_to_gst_fmt( p_dec->fmt_in.i_codec );<br>
> +    if( !p_str ) {<br>
> +        return VLC_EGENERIC;<br>
> +    }<br>
> +<br>
> +    /* Allocate the memory needed to store the decoder's structure */<br>
> +    if( ( p_dec->p_sys = p_sys = calloc( 1, sizeof( *p_sys ) ) ) == NULL )<br>
> +        return VLC_ENOMEM;<br>
> +<br>
> +    /* Set callbacks */<br>
> +    p_dec->pf_decode_video = ( picture_t*( * )( decoder_t*, block_t** ) )<br>
> +        DecodeBlock;<br>
<br>
</div>You should not need to cast here.<br>
</div><div><div>Vikram: Agreed.<br></div><div><div class="h5"><div>
> +<br>
> +    dbin = var_CreateGetBool( p_dec, "use-decodebin" );<br>
> +    msg_Info( p_dec, "Using decodebin? %s", dbin ? "yes ":"no" );<br>
> +<br>
> +    /* Force packetized for now */<br>
> +    p_dec->b_need_packetized = true;<br>
> +<br>
> +    if( p_dec->fmt_in.video.i_width &&<br>
> +            p_dec->fmt_in.video.i_height )<br>
> +    {<br>
> +        gst_structure_set( p_str,<br>
> +                "width", G_TYPE_INT, p_dec->fmt_in.video.i_width,<br>
> +                "height", G_TYPE_INT, p_dec->fmt_in.video.i_height, NULL );<br>
> +    }<br>
> +<br>
> +    if( p_dec->fmt_in.video.i_frame_rate &&<br>
> +            p_dec->fmt_in.video.i_frame_rate_base )<br>
> +    {<br>
> +        gst_structure_set( p_str, "framerate", GST_TYPE_FRACTION,<br>
> +                p_dec->fmt_in.video.i_frame_rate,<br>
> +                p_dec->fmt_in.video.i_frame_rate_base, NULL );<br>
> +    }<br>
> +<br>
> +    if( p_dec->fmt_in.video.i_sar_num &&<br>
> +            p_dec->fmt_in.video.i_sar_den )<br>
> +    {<br>
> +        gst_structure_set( p_str, "pixel-aspect-ratio", GST_TYPE_FRACTION,<br>
> +                p_dec->fmt_in.video.i_sar_num,<br>
> +                p_dec->fmt_in.video.i_sar_den, NULL );<br>
> +    }<br>
> +<br>
> +    if( p_dec->fmt_in.i_extra )<br>
> +    {<br>
> +        GstBuffer* p_buf;<br>
> +<br>
> +        p_data = malloc( p_dec->fmt_in.i_extra );<br>
> +        VLC_GST_CHECK( p_data, NULL, "failed to create codec data buffer",<br>
> +                VLC_ENOMEM, fail );<br>
> +        memcpy( p_data, p_dec->fmt_in.p_extra, p_dec->fmt_in.i_extra );<br>
> +        p_buf = gst_buffer_new_wrapped( p_data, p_dec->fmt_in.i_extra );<br>
> +        VLC_GST_CHECK( p_buf, NULL, "failed to create codec data<br>
> gstbuffer",<br>
> +                VLC_ENOMEM, fail );<br>
> +<br>
> +        gst_structure_set( p_str, "codec_data", GST_TYPE_BUFFER, p_buf,<br>
> NULL );<br>
> +        gst_buffer_unref( p_buf );<br>
> +        p_data = NULL;<br>
> +    }<br>
> +<br>
> +    p_caps = gst_caps_new_empty( );<br>
> +    gst_caps_append_structure( p_caps, p_str );<br>
> +    p_str = NULL;<br>
> +<br>
> +    /* Get the list of all the available gstreamer decoders */<br>
> +    p_list = gst_element_factory_list_get_elements(<br>
> +            GST_ELEMENT_FACTORY_TYPE_DECODER, GST_RANK_MARGINAL );<br>
> +    VLC_GST_CHECK( p_list, NULL, "no decoder list found", VLC_ENOMOD, fail<br>
> ); +    if( !dbin )<br>
> +    {<br>
> +        GList* p_l;<br>
> +        /* Sort them as per ranks */<br>
> +        p_list = g_list_sort( p_list, compare_ranks_func );<br>
> +        VLC_GST_CHECK( p_list, NULL, "failed to sort decoders list",<br>
> +                VLC_ENOMOD, fail );<br>
> +        p_l = g_list_find_custom( p_list, p_caps, find_decoder_func );<br>
> +        VLC_GST_CHECK( p_l, NULL, "no suitable decoder found",<br>
> +                VLC_ENOMOD, fail );<br>
> +        /* create the decoder with highest rank */<br>
> +        p_sys->p_decode_in = gst_element_factory_create(<br>
> +                (GstElementFactory*)p_l->data, NULL );<br>
> +        VLC_GST_CHECK( p_sys->p_decode_in, NULL,<br>
> +                "failed to create decoder", VLC_ENOMOD, fail );<br>
> +    }<br>
> +    else<br>
> +    {<br>
> +        GList* p_l;<br>
> +        /* Just check if any suitable decoder exists, rest will be<br>
> +         * handled by decodebin */<br>
> +        p_l = g_list_find_custom( p_list, p_caps, find_decoder_func );<br>
> +        VLC_GST_CHECK( p_l, NULL, "no suitable decoder found",<br>
> +                VLC_ENOMOD, fail );<br>
> +    }<br>
> +    gst_plugin_feature_list_free( p_list );<br>
> +    p_list = NULL;<br>
> +<br>
> +    vlc_sem_init( &p_sys->sem_mt, 0 );<br>
> +    p_dec->fmt_out.i_cat = VIDEO_ES;<br>
> +<br>
> +    p_sys->b_prerolled = false;<br>
> +    p_sys->b_running = false;<br>
> +    p_sys->b_out_fmt_set = false;<br>
> +<br>
> +    /* Queue: GStreamer thread will dump buffers into this queue,<br>
> +     * DecodeBlock() will pop out the buffers from the queue */<br>
> +    p_sys->p_que = gst_atomic_queue_new( 0 );<br>
> +    VLC_GST_CHECK ( p_sys->p_que, NULL, "failed to create queue",<br>
> +            VLC_ENOMEM, fail );<br>
> +<br>
> +    p_sys->p_decode_src = gst_element_factory_make( "appsrc", NULL );<br>
> +    VLC_GST_CHECK( p_sys->p_decode_src, NULL, "appsrc not found",<br>
> +            VLC_ENOMOD, fail );<br>
> +    g_object_set( G_OBJECT( p_sys->p_decode_src ), "caps", p_caps,<br>
> +            "block", FALSE, "emit-signals", TRUE, "format",<br>
> GST_FORMAT_BYTES, +            "stream-type", GST_APP_STREAM_TYPE_SEEKABLE,<br>
> NULL );<br>
> +    gst_caps_unref( p_caps );<br>
> +    p_caps = NULL;<br>
> +    cb.enough_data = cb.need_data = NULL;<br>
> +    cb.seek_data = seek_data_cb;<br>
> +    gst_app_src_set_callbacks( GST_APP_SRC( p_sys->p_decode_src ),<br>
> +            &cb, p_dec, NULL );<br>
> +<br>
> +    if( dbin )<br>
> +    {<br>
> +        p_sys->p_decode_in = gst_element_factory_make( "decodebin", NULL );<br>
> +        VLC_GST_CHECK( p_sys->p_decode_in, NULL, "decodebin not found", +<br>
>               VLC_ENOMOD, fail );<br>
> +        //g_object_set( G_OBJECT (p_sys->p_decode_in),<br>
> +        //"max-size-buffers", 2, NULL );<br>
> +        g_signal_connect( G_OBJECT( p_sys->p_decode_in ), "pad-added",<br>
> +                G_CALLBACK( pad_added_cb ), p_dec );<br>
> +        g_signal_connect (G_OBJECT( p_sys->p_decode_in ), "no-more-pads",<br>
> +                G_CALLBACK( no_more_pads_cb ), p_dec );<br>
> +    }<br>
> +    p_pad = gst_element_get_static_pad( p_sys->p_decode_in, "sink" );<br>
> +    VLC_GST_CHECK( p_pad, NULL, "failed to get decoder's sinkpad",<br>
> +            VLC_ENOOBJ, fail );<br>
> +    gst_pad_add_probe( p_pad, GST_PAD_PROBE_TYPE_BUFFER, buffer_probe_cb,<br>
> +            p_dec, NULL );<br>
> +    gst_object_unref( p_pad );<br>
> +<br>
> +    /* fakesink: will emit signal for every available buffer */<br>
> +    p_sys->p_decode_out = gst_element_factory_make( "fakesink", NULL );<br>
> +    VLC_GST_CHECK( p_sys->p_decode_out, NULL, "fakesink not found",<br>
> +            VLC_ENOMOD, fail );<br>
> +    /* connect to the signal with the callback */<br>
> +    g_object_set( G_OBJECT( p_sys->p_decode_out ), "sync", FALSE,<br>
> +            "enable-last-sample", FALSE, "signal-handoffs", TRUE, NULL );<br>
> +    g_signal_connect( G_OBJECT( p_sys->p_decode_out ), "handoff",<br>
> +            G_CALLBACK( frame_handoff_cb ), p_dec );<br>
> +<br>
> +    p_sys->p_decoder = GST_ELEMENT( gst_bin_new( "decoder" ) );<br>
> +    VLC_GST_CHECK( p_sys->p_decoder, NULL, "bin not found", VLC_ENOMOD,<br>
> fail ); +    p_sys->p_bus = gst_bus_new( );<br>
> +    VLC_GST_CHECK( p_sys->p_bus, NULL, "failed to create bus",<br>
> +            VLC_ENOMOD, fail );<br>
> +    gst_element_set_bus( p_sys->p_decoder, p_sys->p_bus );<br>
> +<br>
> +    gst_bin_add_many( GST_BIN( p_sys->p_decoder ),<br>
> +            p_sys->p_decode_src, p_sys->p_decode_in,<br>
> +            p_sys->p_decode_out, NULL );<br>
> +    gst_object_ref( p_sys->p_decode_src );<br>
> +    gst_object_ref( p_sys->p_decode_in );<br>
> +    gst_object_ref( p_sys->p_decode_out );<br>
> +<br>
> +    b_ret = gst_element_link( p_sys->p_decode_src, p_sys->p_decode_in );<br>
> +    VLC_GST_CHECK ( b_ret, FALSE, "failed to link src <-> in",<br>
> +            VLC_EGENERIC, fail );<br>
> +<br>
> +    if( !dbin )<br>
> +    {<br>
> +        b_ret = gst_element_link( p_sys->p_decode_in, p_sys->p_decode_out<br>
> ); +        VLC_GST_CHECK (b_ret, FALSE, "failed to link in <-> out", +<br>
>            VLC_EGENERIC, fail);<br>
> +    }<br>
> +<br>
> +    /* set the pipeline to playing */<br>
> +    i_ret = gst_element_set_state( p_sys->p_decoder, GST_STATE_PLAYING );<br>
> +    VLC_GST_CHECK( i_ret, GST_STATE_CHANGE_FAILURE,<br>
> +            "set state failure", VLC_EGENERIC, fail );<br>
> +<br>
> +    p_sys->b_running = true;<br>
> +    p_sys->b_new_segment_pending = false;<br>
> +<br>
> +    return VLC_SUCCESS;<br>
> +<br>
> +fail:<br>
> +    if( p_data )<br>
<br>
</div></div></div></div>Not needed.<br></blockquote><div>   Vikram: ok. <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div><div class="h5">

<div><div><br>
> +        free( p_data );<br>
> +    if( p_caps )<br>
> +    {<br>
> +        gst_caps_unref( p_caps );<br>
> +        p_str = NULL;<br>
> +    }<br>
> +    if( p_str )<br>
> +        gst_structure_free( p_str );<br>
> +    if( p_list )<br>
> +        gst_plugin_feature_list_free( p_list );<br>
> +    CloseDecoder( ( vlc_object_t* )p_dec );<br>
> +    return i_rval;<br>
> +}<br>
> +<br>
> +/* Decode */<br>
> +static picture_t *DecodeBlock( decoder_t *p_dec, block_t **pp_block )<br>
> +{<br>
> +    block_t *p_block = NULL;<br>
> +    picture_t *p_pic = NULL;<br>
> +    decoder_sys_t *p_sys = p_dec->p_sys;<br>
> +    GstMessage* p_msg;<br>
> +    gboolean b_ret;<br>
> +    guint8* p_data;<br>
> +    GstBuffer* p_buf;<br>
> +<br>
> +    if( !pp_block || !*pp_block ) {<br>
> +        return NULL;<br>
> +    }<br>
> +<br>
> +    p_block = *pp_block;<br>
> +<br>
> +    if( p_block->i_flags & (BLOCK_FLAG_DISCONTINUITY|BLOCK_FLAG_CORRUPTED)<br>
> ) +    {<br>
> +        if ( p_block->i_flags & BLOCK_FLAG_DISCONTINUITY )<br>
> +            p_sys->b_new_segment_pending = true;<br>
> +<br>
> +        block_Release( p_block );<br>
> +        return NULL;<br>
> +    }<br>
> +<br>
> +    if ( !p_block->i_buffer )<br>
> +    {<br>
> +        block_Release( p_block );<br>
> +        return NULL;<br>
> +    }<br>
> +<br>
> +    p_data = malloc (p_block->i_buffer);<br>
> +    if( unlikely( p_data == NULL ) )<br>
> +    {<br>
> +        block_Release( p_block );<br>
> +        msg_Err( p_dec, "failed to create buffer" );<br>
> +        p_dec->b_error = true;<br>
> +        goto done;<br>
> +    }<br>
> +    memcpy( p_data, p_block->p_buffer, p_block->i_buffer );<br>
> +    p_buf = gst_buffer_new_wrapped( p_data, p_block->i_buffer );<br>
<br>
</div></div>No. Wrapping a VLC block_t into a GstBuffer is not that difficult. There is no<br>
need for memory copying. I already wrote that code a few years ago.<br>
</div></div><div><div>Vikram: yes, have removed memcpying  in my local patch here.<br></div></div></blockquote><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

<div><div>
> +    if( unlikely( p_buf == NULL ) )<br>
> +    {<br>
> +        free( p_data );<br>
> +        block_Release( p_block );<br>
> +        msg_Err( p_dec, "failed to create gstbuffer" );<br>
> +        p_dec->b_error = true;<br>
> +        goto done;<br>
> +    }<br>
> +<br>
> +    if( p_block->i_dts > VLC_TS_INVALID )<br>
> +        GST_BUFFER_DTS( p_buf ) = gst_util_uint64_scale( p_block->i_dts,<br>
> +                GST_SECOND, GST_MSECOND );<br>
> +<br>
> +    if( p_block->i_pts <= VLC_TS_INVALID )<br>
> +        GST_BUFFER_PTS( p_buf ) = GST_BUFFER_DTS( p_buf );<br>
> +    else<br>
> +        GST_BUFFER_PTS( p_buf ) = gst_util_uint64_scale( p_block->i_pts,<br>
> +                GST_SECOND, GST_MSECOND );<br>
> +<br>
> +    if( p_dec->fmt_in.video.i_frame_rate  &&<br>
> +            p_dec->fmt_in.video.i_frame_rate_base )<br>
> +        GST_BUFFER_DURATION( p_buf ) = gst_util_uint64_scale( GST_SECOND,<br>
> +                p_dec->fmt_in.video.i_frame_rate_base,<br>
> +                p_dec->fmt_in.video.i_frame_rate );<br>
> +<br>
> +    /* Send a new segment event, seeking position is<br>
> +     * irrelavant here, as the main motive for a seek<br>
> +     * here, is to tell the elements to start flushing<br>
> +     * and start accepting buffers from a new time segment */<br>
> +    if( unlikely( p_sys->b_new_segment_pending ) )<br>
> +    {<br>
> +        GstBuffer* p_buffer;<br>
> +<br>
> +        b_ret = gst_element_seek_simple( p_sys->p_decoder,<br>
> +                GST_FORMAT_BYTES, GST_SEEK_FLAG_FLUSH, 0 );<br>
> +        msg_Dbg( p_dec, "new segment event : %d", b_ret );<br>
> +<br>
> +        /* flush the ouyput buffers from the queue */<br>
> +        while( ( p_buffer = gst_atomic_queue_pop( p_sys->p_que ) ) )<br>
> +            gst_buffer_unref ( p_buffer );<br>
> +<br>
> +        p_sys->b_new_segment_pending = false;<br>
> +        p_sys->b_prerolled = false;<br>
> +    }<br>
> +<br>
> +    /* Give the input buffer to GStreamer Bin.<br>
> +     *<br>
> +     *  libvlc                     libvlc<br>
> +     *    \ (i/p)              (o/p) ^<br>
> +     *     \                        /<br>
> +     *   ___v____GSTREAMER BIN_____/____<br>
> +     *  |                               |<br>
> +     *  |   appsrc-->decode-->fakesink  |<br>
> +     *  |_______________________________|<br>
> +     *<br>
> +     * * * * * * * * * * * * * * * * * * * * */<br>
> +    if( unlikely( gst_app_src_push_buffer(<br>
> +                    GST_APP_SRC_CAST( p_sys->p_decode_src ), p_buf )<br>
> +                != GST_FLOW_OK ) )<br>
> +    {<br>
> +        p_dec->b_error = true;<br>
> +        msg_Err( p_dec, "failed to push buffer" );<br>
> +        goto done;<br>
> +    }<br>
> +    else<br>
> +    {<br>
> +        vlc_sem_wait( &p_sys->sem_mt );<br>
> +    }<br>
> +<br>
> +    do<br>
> +    {<br>
> +        /* Poll for any messages, errors */<br>
> +        p_msg = gst_bus_pop_filtered( p_sys->p_bus,<br>
> +                GST_MESSAGE_ASYNC_DONE | GST_MESSAGE_ERROR |<br>
> +                GST_MESSAGE_EOS | GST_MESSAGE_APPLICATION |<br>
> +                GST_MESSAGE_WARNING | GST_MESSAGE_INFO );<br>
<br>
</div></div>Won't we get stuck in vlc_sem_wait() before this code has a chance to execute?<br></blockquote></div></div><div>   Vikram; Not unless appsrc encounters an error (which will not happen in this case). Basically here, I wanted to synchronize the flow for every buffer from vlc->appsrc and  then <br>

               appsrc->decoder. Not synchronizing this flow causes DecodeBlock() to rapidly dump the input buffers to the appsrc causing high latency (since appsrc starts it's own <br>               thread to push buffers downstream) at the output and then subsequently video frame drops. <br>

</div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div><div><br>
> +        if( p_msg )<br>
> +        {<br>
> +            switch ( GST_MESSAGE_TYPE( p_msg ) )<br>
> +            {<br>
> +                case GST_MESSAGE_ERROR:<br>
> +                    {<br>
> +                        gchar  *psz_debug;<br>
> +                        GError *p_error;<br>
> +<br>
> +                        gst_message_parse_error (p_msg, &p_error,<br>
> &psz_debug);<br>
> +                        g_free (psz_debug);<br>
> +<br>
> +                        msg_Err( p_dec, "Error from %s: %s",<br>
> +                                GST_ELEMENT_NAME( GST_MESSAGE_SRC( p_msg )<br>
> ),<br>
> +                                p_error->message );<br>
> +                        g_error_free( p_error );<br>
> +                        p_dec->b_error = true;<br>
> +                    }<br>
> +                    break;<br>
> +                case GST_MESSAGE_EOS:<br>
> +                    msg_Warn( p_dec, "got unexpected eos" );<br>
> +                    break;<br>
> +                /* First buffer received */<br>
> +                case GST_MESSAGE_ASYNC_DONE:<br>
> +                    p_sys->b_prerolled = true;<br>
> +                    msg_Dbg( p_dec, "Pipeline is prerolled" );<br>
> +                    break;<br>
> +                case GST_MESSAGE_APPLICATION:<br>
> +                    {<br>
> +                        const GstStructure *p_str;<br>
> +                        gboolean b_err, b_warn;<br>
> +<br>
> +                        p_str = gst_message_get_structure ( p_msg );<br>
> +                        if( p_str &&<br>
> +                                ( (<br>
> +                                   b_err = gst_structure_has_name(<br>
> +                                       p_str,<br>
> +                                       "VlcGstError"<br>
> +                                       )<br>
> +                                  ) ||<br>
> +                                  (<br>
> +                                   b_warn = gst_structure_has_name(<br>
> +                                       p_str,<br>
> +                                       "VlcGstWarning"<br>
> +                                       )<br>
> +                                  ) ) )<br>
> +                        {<br>
> +                            const gchar* psz_str;<br>
> +<br>
> +                            psz_str = gst_structure_get_string( p_str,<br>
> +                                    "message" );<br>
> +<br>
> +                            if( b_err )<br>
> +                            {<br>
> +                                msg_Err( p_dec, psz_str );<br>
> +                                p_dec->b_error = true;<br>
> +                            }<br>
> +                            else if( b_warn )<br>
> +                                msg_Warn( p_dec, psz_str );<br>
> +                        }<br>
> +                    }<br>
> +                    break;<br>
> +                case GST_MESSAGE_INFO:<br>
> +                    {<br>
> +                        gchar  *psz_debug;<br>
> +                        GError *p_error;<br>
> +<br>
> +                        gst_message_parse_info( p_msg, &p_error, &psz_debug<br>
> ); +                        g_free( psz_debug );<br>
> +<br>
> +                        msg_Info( p_dec, "Info from %s: %s",<br>
> +                                GST_ELEMENT_NAME( GST_MESSAGE_SRC( p_msg )<br>
> ), +                                p_error->message );<br>
> +                        g_error_free( p_error );<br>
> +                    }<br>
> +                    break;<br>
> +                case GST_MESSAGE_WARNING:<br>
> +                    {<br>
> +                        gchar  *psz_debug;<br>
> +                        GError *p_error;<br>
> +<br>
> +                        gst_message_parse_info( p_msg, &p_error, &psz_debug<br>
> );<br>
> +                        g_free( psz_debug );<br>
> +<br>
> +                        msg_Warn( p_dec, "Warning from %s: %s",<br>
> +                                GST_ELEMENT_NAME( GST_MESSAGE_SRC( p_msg )<br>
> ),<br>
> +                                p_error->message );<br>
> +                        g_error_free( p_error );<br>
> +                    }<br>
> +                    break;<br>
> +                default:<br>
> +                    break;<br>
<br>
</div></div>msg_*() is thread-safe. You don't need to defer and serialize calls to logging<br>
functions. (And for the sake of debugging, you probably should not defer<br>
those.)<br></blockquote></div></div><div>   Vikram: Ok, I'll remove posting the application messages and directly print them at the occurrence point.<br></div><div><div class="h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<div><div><br>
> +            }<br>
> +            gst_message_unref( p_msg );<br>
> +        }<br>
> +<br>
> +        /* Look for any output buffers in the queue */<br>
> +        if( gst_atomic_queue_peek( p_sys->p_que ) ) {<br>
> +            /* Get a new picture */<br>
> +            p_pic = decoder_NewPicture( p_dec );<br>
> +<br>
> +            if( p_pic )<br>
> +            {<br>
> +                GstBuffer* p_buf = GST_BUFFER_CAST(<br>
> +                        gst_atomic_queue_pop( p_sys->p_que ) );<br>
> +                GstVideoFrame frame;<br>
> +<br>
> +                if( likely( GST_BUFFER_PTS_IS_VALID( p_buf ) ) )<br>
> +                {<br>
> +                    p_pic->date = gst_util_uint64_scale(<br>
> +                            GST_BUFFER_PTS( p_buf ), GST_MSECOND,<br>
> GST_SECOND ); +                }<br>
> +<br>
> +                if( unlikely( !gst_video_frame_map( &frame,<br>
> +                                &p_sys->vinfo, p_buf, GST_MAP_READ ) ) )<br>
> +                {<br>
> +                    msg_Err( p_dec, "failed to map gst video frame" );<br>
> +                    gst_buffer_unref( p_buf );<br>
> +                    p_dec->b_error = true;<br>
> +                    break;<br>
> +<br>
> +                }<br>
> +                else<br>
> +                {<br>
> +                    gst_CopyPicture( p_pic, &frame );<br>
> +                    gst_video_frame_unmap( &frame );<br>
> +                }<br>
> +<br>
> +                gst_buffer_unref( p_buf );<br>
> +            } else<br>
> +                break;<br>
> +        }<br>
> +<br>
> +    } while (0);<br>
> +<br>
> +done:<br>
> +    block_Release( p_block );<br>
> +    *pp_block = NULL;<br>
> +    return p_pic;<br>
> +}<br>
> +<br>
> +/* Close the decoder instance */<br>
> +static void CloseDecoder( vlc_object_t *p_this )<br>
> +{<br>
> +    decoder_t *p_dec = ( decoder_t *)p_this;<br>
> +    decoder_sys_t *p_sys = p_dec->p_sys;<br>
> +<br>
> +     if( p_sys == NULL )<br>
> +             return;<br>
> +<br>
> +    if( p_sys->b_running )<br>
> +    {<br>
> +        GstMessage* p_msg;<br>
> +        GstFlowReturn i_ret;<br>
> +<br>
> +        /* Send EOS to the pipeline */<br>
> +        i_ret = gst_app_src_end_of_stream(<br>
> +                GST_APP_SRC_CAST( p_sys->p_decode_src ) );<br>
> +        msg_Dbg( p_dec, "app src eos: %s", gst_flow_get_name( i_ret ) );<br>
> +<br>
> +        /* and catch it on the bus */<br>
> +        p_msg = gst_bus_timed_pop_filtered( p_sys->p_bus,<br>
> +                2000000000ULL, GST_MESSAGE_EOS | GST_MESSAGE_ERROR );<br>
> +<br>
> +        if( p_msg )<br>
> +        {<br>
> +            switch ( GST_MESSAGE_TYPE( p_msg ) )<br>
> +            {<br>
> +                case GST_MESSAGE_ERROR:<br>
> +                    {<br>
> +                        gchar  *psz_debug;<br>
> +                        GError *p_error;<br>
> +<br>
> +                        gst_message_parse_error ( p_msg, &p_error,<br>
> &psz_debug ); +                        g_free( psz_debug );<br>
> +<br>
> +                        msg_Err( p_dec, "Error from %s: %s",<br>
> +                                GST_ELEMENT_NAME( GST_MESSAGE_SRC( p_msg )<br>
> ), +                                p_error->message );<br>
> +                        g_error_free( p_error );<br>
> +                    }<br>
> +                    break;<br>
> +<br>
> +                case GST_MESSAGE_EOS:<br>
> +                    msg_Dbg( p_dec, "got eos" );<br>
> +                    break;<br>
> +<br>
> +                default:<br>
> +                    break;<br>
> +            }<br>
> +<br>
> +            gst_message_unref( p_msg );<br>
> +        }<br>
> +        else<br>
> +        {<br>
> +            msg_Warn( p_dec, "failed to get message" );<br>
> +        }<br>
> +    }<br>
> +<br>
> +    /* Remove any left-over buffers from the queue */<br>
> +    if( p_sys->p_que )<br>
> +    {<br>
> +        GstBuffer* p_buf;<br>
> +        while( ( p_buf = gst_atomic_queue_pop( p_sys->p_que ) ) )<br>
> +            gst_buffer_unref( p_buf );<br>
> +        gst_atomic_queue_unref( p_sys->p_que );<br>
> +    }<br>
> +<br>
> +    if( p_sys->b_running )<br>
> +    {<br>
> +        p_sys->b_running = false;<br>
> +        gst_element_set_state ( p_sys->p_decoder, GST_STATE_NULL );<br>
> +    }<br>
> +<br>
> +    if( p_sys->p_bus )<br>
> +        gst_object_unref( p_sys->p_bus );<br>
> +    if( p_sys->p_decode_src )<br>
> +        gst_object_unref( p_sys->p_decode_src );<br>
> +    if( p_sys->p_decode_in )<br>
> +        gst_object_unref( p_sys->p_decode_in );<br>
> +    if( p_sys->p_decode_out )<br>
> +        gst_object_unref( p_sys->p_decode_out );<br>
> +    if( p_sys->p_decoder )<br>
> +        gst_object_unref( p_sys->p_decoder);<br>
> +<br>
> +    vlc_sem_destroy( &p_sys->sem_mt );<br>
> +    free( p_sys );<br>
> +    p_dec->p_sys = NULL;<br>
<br>
</div></div>Useless.<br></blockquote></div></div><div>   Vikram: I hope you meant setting p_sys to NULL here and not the entire code :-) (Fingers crossed!) <br></div><div class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<br>
> +}<br>
<span><font color="#888888"><br>
--<br>
Rémi Denis-Courmont<br>
<a href="http://www.remlab.net/" target="_blank">http://www.remlab.net/</a><br>
</font></span><div><div><br>
_______________________________________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="https://mailman.videolan.org/listinfo/vlc-devel" target="_blank">https://mailman.videolan.org/listinfo/vlc-devel</a><br>
</div></div></blockquote></div></div><br></div></div>
</blockquote></div><br></div></div></div>