[vlc-devel] [PATCH 1/1] codec: Add GStreamer based video decoder module

ved kpl ved.kpl at gmail.com
Mon Apr 21 19:33:47 CEST 2014


Thanks Remi. Comments inline.


On Mon, Apr 21, 2014 at 9:15 PM, Rémi Denis-Courmont <remi at remlab.net>wrote:

> Le samedi 19 avril 2014, 13:51:56 Vikram Fugro a écrit :
> > This module currently supports h264, mpeg4,
> > vp8, mpeg2, flashvideo, wmv1/2/3, vc1 decoder
> > plugins provided by GStreamer 1.0 library.
> >
> > Signed-off-by: Vikram Fugro <ved.kpl at gmail.com>
> > ---
> >  configure.ac              |    5 +
> >  modules/codec/Makefile.am |    7 +
> >  modules/codec/gstcodec.c  |  998
> > +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 1010
> > insertions(+)
> >  create mode 100644 modules/codec/gstcodec.c
> >
> > diff --git a/configure.ac b/configure.ac
> > index 2f28385..5dc23e4 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -2707,6 +2707,11 @@ dnl
> >  PKG_ENABLE_MODULES_VLC([THEORA], [], [ogg theoradec >= 1.0 theoraenc],
> > [experimental theora codec], [auto])
> >
> >  dnl
> > +dnl GStreamer plugins
> > +dnl
> > +PKG_ENABLE_MODULES_VLC([GSTREAMER], [gstcodec], [gstreamer-1.0
> > gstreamer-base-1.0 gstreamer-app-1.0 gstreamer-basevideo-1.0
> > gstreamer-video-1.0], [gstreamer], [auto])
>
> I can't find some of those anywhere. Also I am not sure merging generic and
> video libraries in a single variable is a good idea.
> Vikram: Which ones? You suggest to have more than one PKG_ENABLE_MODULES?
>


> > +
> > +dnl
> >  dnl  schroedinger decoder plugin (for dirac format video)
> >  dnl
> >  PKG_ENABLE_MODULES_VLC([SCHROEDINGER], [], [schroedinger-1.0 >= 1.0.10],
> > [dirac decoder and encoder using schroedinger], [auto])
> > diff --git
> > a/modules/codec/Makefile.am b/modules/codec/Makefile.am index
> > 14a53fd..0c0de5e 100644
> > --- a/modules/codec/Makefile.am
> > +++ b/modules/codec/Makefile.am
> > @@ -464,3 +464,10 @@ libquicktime_plugin_la_LDFLAGS = $(AM_LDFLAGS)
> -rpath
> > '$(codecdir)' libquicktime_plugin_la_LIBADD = $(LIBM)
> >  EXTRA_LTLIBRARIES += libquicktime_plugin.la
> >  codec_LTLIBRARIES += $(LTLIBquicktime)
> > +
> > +libgstcodec_plugin_la_SOURCES = codec/gstcodec.c
> > +libgstcodec_plugin_la_CFLAGS = $(AM_CFLAGS) $(GSTREAMER_CFLAGS)
> > +libgstcodec_plugin_la_LDFLAGS = $(AM_LDFLAGS) -rpath '$(codecdir)'
> > +libgstcodec_plugin_la_LIBADD = $(GSTREAMER_LIBS)
> > +EXTRA_LTLIBRARIES += libgstcodec_plugin.la
> > +codec_LTLIBRARIES += $(LTLIBgstcodec)
> > diff --git a/modules/codec/gstcodec.c b/modules/codec/gstcodec.c
> > new file mode 100644
> > index 0000000..43319a7
> > --- /dev/null
> > +++ b/modules/codec/gstcodec.c
> > @@ -0,0 +1,998 @@
> >
> +/**************************************************************************
> > *** + * gstcodec.c: video decoder module making use of gstreamer
> > +
> >
> ***************************************************************************
> > ** + * Copyright (C) 1999-2014 the VideoLAN team
>
> 1999?
> Vikram: Yeah, my mistake! it should just be 2014, right?
>


> > + * $Id:
> > + *
> > + * Author: Vikram Fugro <ved.kpl at gmail.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA
> 02110-1301,
> > USA. +
> >
> ***************************************************************************
> > **/ +
> >
> +/**************************************************************************
> > *** + * Preamble
> > +
> >
> ***************************************************************************
> > **/ +#ifdef HAVE_CONFIG_H
> > +# include "config.h"
> > +#endif
> > +
> > +#include <vlc_common.h>
> > +#include <vlc_plugin.h>
> > +#include <vlc_codec.h>
> > +#include <vlc_sout.h>
> > +#include <vlc_input.h>
> > +
> > +#include <assert.h>
> > +#include <limits.h>
> > +
> > +#include <gst/gst.h>
> > +#include <gst/video/gstvideometa.h>
> > +#include <gst/video/video.h>
> > +#include <gst/app/gstappsrc.h>
> > +#include <gst/gstatomicqueue.h>
> > +
> > +struct decoder_sys_t
> > +{
> > +    GstElement* p_decoder;
> > +    GstElement* p_decode_src;
> > +    GstElement* p_decode_in;
> > +    GstElement* p_decode_out;
> > +
> > +    GstBus* p_bus;
> > +
> > +    GstVideoInfo vinfo;
> > +    GstAtomicQueue *p_que;
> > +    bool b_prerolled;
> > +    bool b_running;
> > +
> > +    bool b_new_segment_pending;
> > +    bool b_out_fmt_set;
> > +    vlc_sem_t sem_mt;
> > +};
> > +
> >
> +/**************************************************************************
> > *** + * Local prototypes
> > +
> >
> ***************************************************************************
> > **/ +static int  OpenDecoder( vlc_object_t * );
> > +static void CloseDecoder( vlc_object_t * );
> > +static picture_t *DecodeBlock( decoder_t *, block_t ** );
> > +
> > +#define MODULE_DESCRIPTION N_( "Various video decoders " \
> > +        "delivered by the GStreamer library.")
> > +
> > +#define USEDECODEBIN_TEXT N_("Use DecodeBin")
> > +#define USEDECODEBIN_LONGTEXT N_( \
> > +    "Let DecodeBin choose the decoder." )
>
> So hmm, why should the user have the choice and on which basis should (s)he
> make that choice?
>
> In my limited understanding of gstreamer, decodebin provides demux,
> packetizer
> and decoder together as one wrapper element. Is there any benefit to
> decodebin
> for a VLC decoder plugin? Conversely, is there any disadvantage to calling
> the
> specific gstreamer decoder elements directly?
> 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.
>
                If you think it is not really needed, I 'll have it
removed.


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

>
> > +
> > +/* Probe for the buffer just pushed into appsrc
> > + * Maintains sync between the DecodeBlock() and
> > + * the appsrc */
> > +static GstPadProbeReturn buffer_probe_cb( GstPad* p_pad,
> > +        GstPadProbeInfo* p_info, gpointer p_data )
> > +{
> > +    decoder_t* p_dec = ( decoder_t* )p_data;
> > +    vlc_sem_post( &p_dec->p_sys->sem_mt );
> > +    return GST_PAD_PROBE_OK;
> > +}
> > +
> > +/* Emitted by appsrc when serving a seek request.
> > + * Seek over here is only used for flushing the buffers.
> > + * Returns TRUE always, as the 'real' seek will be
> > + * done by VLC framework */
>
> I am not sure that makes much sense. I mean, can this event ever happen
> when
> the decoder is between an appsink and an appsrc elements?
>
   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.

>
> > +static gboolean seek_data_cb( GstAppSrc* p_src, guint64 l_offset,
> > +        gpointer p_data )
> > +{
> > +    decoder_t* p_dec = ( decoder_t* )p_data;
> > +    msg_Dbg( p_dec, "appsrc seeking to %llu", (unsigned long
> long)l_offset
> > );
> > +    return TRUE;
> > +}
> > +
> > +/* Emitted by decodebin when there are no more
> > + * outputs. In this case it will be mostly and
> > + * should be one */
> > +static void no_more_pads_cb( GstElement* p_ele, gpointer p_data )
> > +{
> > +    decoder_t* p_dec = ( decoder_t* )p_data;
> > +    GstPad* p_pad;
> > +
> > +    msg_Dbg( p_dec, "no more pads" );
> > +
> > +    p_pad = gst_element_get_static_pad( p_dec->p_sys->p_decode_out,
> > +            "sink" );
> > +    if( p_pad == NULL || !gst_pad_is_linked( p_pad ) )
> > +    {
> > +        gst_element_post_message ( GST_ELEMENT( p_dec->p_sys->p_decoder
> ),
> > +                gst_message_new_application( GST_OBJECT(
> > +                        p_dec->p_sys->p_decoder ),
> > +                    gst_structure_new( "VlcGstError",
> > +                        "message", G_TYPE_STRING,
> > +                        "failed to link decode out pad",
> > +                        NULL ) ) );
> > +    }
> > +
> > +    gst_object_unref( p_pad );
> > +}
> > +
> > +/* Emitted by decodebin and links decodebin to fakesink*/
> > +static void pad_added_cb( GstElement* p_ele, GstPad* p_pad, gpointer
> p_data
> > )
> > +{
> > +    decoder_t* p_dec = ( decoder_t* )p_data;
> > +    GstPad* p_sinkpad;
> > +
> > +    p_sinkpad = gst_element_get_static_pad(
> > +            p_dec->p_sys->p_decode_out, "sink" );
> > +
> > +    if( !gst_pad_is_linked( p_sinkpad ) )
> > +    {
> > +        GstCaps* p_caps;
> > +
> > +        /* Get the output format type */
> > +        p_caps = gst_pad_get_current_caps( p_pad );
> > +        if( p_caps )
> > +        {
> > +            GstStructure* p_str;
> > +
> > +            p_str = gst_caps_get_structure( p_caps, 0 );
> > +
> > +            /* We want raw caps only */
> > +            if( gst_structure_has_name( p_str, "video/x-raw" ) ) {
> > +                if( !gst_video_info_from_caps( &p_dec->p_sys->vinfo,
> > +                            p_caps ) )
> > +                {
> > +                    gst_element_post_message(
> > +                            GST_ELEMENT( p_dec->p_sys->p_decoder ),
> > +                            gst_message_new_application( GST_OBJECT(
> > +                                    p_dec->p_sys->p_decoder ),
> > +                                gst_structure_new( "VlcGstWarning",
> > +                                    "message", G_TYPE_STRING,
> > +                                    "failed to get video info from
> caps",
> > +                                    NULL ) ) );
> > +                } else
> > +                    gst_pad_link( p_pad, p_sinkpad );
> > +            }
> > +
> > +            gst_caps_unref( p_caps );
> > +        }
> > +    }
> > +
> > +    gst_object_unref( p_sinkpad );
> > +}
> > +
> > +/* Emitted by fakesink for every buffer and sets the
> > + * output format in VLC context for vout. Adds the
> > + * buffer to the queue */
> > +static void frame_handoff_cb( GstElement* p_ele, GstBuffer* p_buf,
> > +        GstPad* p_pad, gpointer p_data )
> > +{
> > +    decoder_t* p_dec = ( decoder_t* )p_data;
> > +
> > +    if( unlikely( p_dec->p_sys->b_out_fmt_set == false ) )
> > +    {
> > +        do
> > +        {
> > +            if( gst_pad_has_current_caps( p_pad ) )
> > +            {
> > +                GstCaps* p_caps;
> > +
> > +                p_caps = gst_pad_get_current_caps( p_pad );
> > +                if( p_caps )
> > +                {
> > +                    GstStructure* p_str;
> > +
> > +                    p_str = gst_caps_get_structure( p_caps, 0 );
> > +                    if( p_str && gst_structure_has_name( p_str,
> > "video/x-raw" ) )
> > +                    {
> > +                        gboolean b_ret = FALSE;
> > +
> > +                        if( !gst_video_info_from_caps(
> > &p_dec->p_sys->vinfo,
> > +                                    p_caps ) )
> > +                        {
> > +                            gst_element_post_message(
> > +                                    GST_ELEMENT(
> p_dec->p_sys->p_decoder ),
> > +                                    gst_message_new_application(
> > GST_OBJECT( +
> > p_dec->p_sys->p_decoder ), +
> > gst_structure_new( "VlcGstWarning", +
> >      "message", G_TYPE_STRING, +
> > "failed to get video info from caps", +
> >        NULL ) ) );
> > +                            gst_caps_unref( p_caps );
> > +                            break;
> > +                        }
> > +                        else
> > +                        {
> > +                            p_dec->fmt_out.i_codec = gst_to_vlc_fmt(
> > +                                    gst_structure_get_string ( p_str,
> > "format" ) ); +
> > +                            if( !p_dec->fmt_out.i_codec )
> > +                            {
> > +                                gst_element_post_message(
> > +                                        GST_ELEMENT(
> > p_dec->p_sys->p_decoder ), +
> > gst_message_new_application( GST_OBJECT( +
> >               p_dec->p_sys->p_decoder ), +
> >           gst_structure_new( "VlcGstWarning", +
> >                    "message", G_TYPE_STRING, +
> >                   "invalid output format", +
> >                 NULL ) ) );
> > +                                gst_caps_unref( p_caps );
> > +                                break;
> > +                            }
> > +
> > +                            b_ret = gst_structure_get_fraction( p_str,
> > +                                    "pixel-aspect-ratio",
> > +                                    ( gint*
> > )&p_dec->fmt_out.video.i_sar_num, +                                    (
> > gint* )&p_dec->fmt_out.video.i_sar_den ); +
> > +                            if( !b_ret ||
> !p_dec->fmt_out.video.i_sar_num
> > || +                                    !p_dec->fmt_out.video.i_sar_den
> ) +
> >                            {
> > +                                p_dec->fmt_out.video.i_sar_num = 1;
> > +                                p_dec->fmt_out.video.i_sar_den = 1;
> > +                            }
> > +
> > +                            b_ret = gst_structure_get_int ( p_str,
> "width",
> > +                                    ( gint*
> )&p_dec->fmt_out.video.i_width
> > ); +                            b_ret &= gst_structure_get_int ( p_str,
> > "height", +                                    ( gint*
> > )&p_dec->fmt_out.video.i_height ); +
> > +                            if( !b_ret || !p_dec->fmt_out.video.i_width
> ||
> > +                                    !p_dec->fmt_out.video.i_height )
> > +                            {
> > +                                p_dec->fmt_out.video.i_width =
> > +                                    p_dec->fmt_in.video.i_width;
> > +                                p_dec->fmt_out.video.i_height =
> > +                                    p_dec->fmt_in.video.i_height;
> > +                            }
> > +
> > +                            b_ret = gst_structure_get_fraction ( p_str,
> > "framerate", +                                    ( gint*
> > )&p_dec->fmt_out.video.i_frame_rate, +
>  (
> > gint* )&p_dec->fmt_out.video.i_frame_rate_base +
> >         );
> > +
> > +                            if( !b_ret ||
> > !p_dec->fmt_out.video.i_frame_rate || +
> > !p_dec->fmt_out.video.i_frame_rate_base ) +                            {
> > +                                p_dec->fmt_out.video.i_frame_rate =
> > +                                    p_dec->fmt_in.video.i_frame_rate;
> > +                                p_dec->fmt_out.video.i_frame_rate_base =
> > +
>  p_dec->fmt_in.video.i_frame_rate_base;
> > +                            }
> > +                        }
> > +                    }
>
> I think you should split the GstVideoInfo->video_format_t conversion to a
> separate function.
>
   Vikram: Agreed.

>
> > +                    gst_caps_unref( p_caps );
> > +                    p_dec->p_sys->b_out_fmt_set = true;
> > +                }
> > +            }
> > +        } while( 0 );
> > +
> > +        if( !p_dec->p_sys->b_out_fmt_set )
> > +            return;
> > +    }
> > +
> > +    /* Push the buffer to the queue */
> > +    gst_atomic_queue_push( p_dec->p_sys->p_que, gst_buffer_ref( p_buf )
> );
> > +}
> > +
> > +/* Copy the frame data from the GstBuffer (from decoder)
> > + * to the picture obtained from downstream in VLC */
> > +static void gst_CopyPicture( picture_t *p_pic, GstVideoFrame *frame )
> > +{
> > +    int i_plane, i_planes, i_line, i_dst_stride, i_src_stride;
> > +    uint8_t *p_dst, *p_src;
> > +    int i_w, i_h;
> > +
> > +    i_planes = p_pic->i_planes < 3 ? p_pic->i_planes : 3;
> > +    for( i_plane = 0; i_plane < i_planes; i_plane++ )
> > +    {
> > +        p_dst = p_pic->p[i_plane].p_pixels;
> > +        p_src = GST_VIDEO_FRAME_PLANE_DATA( frame, i_plane );
> > +        i_dst_stride  = p_pic->p[i_plane].i_pitch;
> > +        i_src_stride  = GST_VIDEO_FRAME_PLANE_STRIDE( frame, i_plane );
> > +
> > +        i_w = GST_VIDEO_FRAME_COMP_WIDTH( frame,
> > +                i_plane ) * GST_VIDEO_FRAME_COMP_PSTRIDE( frame,
> i_plane );
> > +        i_h = GST_VIDEO_FRAME_COMP_HEIGHT( frame, i_plane );
> > +
> > +        for( i_line = 0;
> > +                i_line < __MIN( p_pic->p[i_plane].i_lines, i_h );
> > +                i_line++ )
> > +        {
> > +            memcpy( p_dst, p_src, i_w );
> > +            p_src += i_src_stride;
> > +            p_dst += i_dst_stride;
> > +        }
> > +    }
> > +}
>
> OK in the beginning, but zero-copy is highly desirable here.
> Vikram: Definitely!! That is _THE_ most important thing and will need
> careful handling.
>
> > +
> > +/* Check if the element can use this caps */
> > +static gint find_decoder_func( gconstpointer p_p1, gconstpointer p_p2 )
> > +{
> > +    GstElementFactory *p_factory;
> > +    GstCaps* p_caps;
> > +
> > +    p_factory = ( GstElementFactory* )p_p1;
> > +    p_caps = ( GstCaps * )p_p2;
> > +
> > +    return( !gst_element_factory_can_sink_any_caps( p_factory, p_caps )
> );
> > +}
> > +
> > +static gint compare_ranks_func( gconstpointer p_p1, gconstpointer p_p2 )
> > +{
> > +    return gst_plugin_feature_rank_compare_func( p_p1, p_p2 );
> > +}
> > +
> >
> +/**************************************************************************
> > *** + * OpenDecoder: probe the decoder and return score
> > +
> >
> ***************************************************************************
> > **/
> > +static int OpenDecoder( vlc_object_t *p_this )
> > +{
> > +    decoder_t *p_dec = ( decoder_t* )p_this;
> > +    decoder_sys_t *p_sys;
> > +    GstStateChangeReturn i_ret;
> > +    gboolean b_ret;
> > +    GstPad* p_pad;
> > +    GstCaps* p_caps = NULL;
> > +    GstStructure* p_str = NULL;
> > +    GstAppSrcCallbacks cb;
> > +    guint8* p_data = NULL;
> > +    int i_rval = VLC_SUCCESS;
> > +    GList* p_list = NULL;
> > +    bool dbin;
> > +#define VLC_GST_CHECK(r, v, s, t, x) \
> > +    { if(r == v) { msg_Info(p_dec, s); i_rval = t; goto x; } }
>
> x is a constant here... But IMHO, goto macros are evil.
> Vikram: I agree, but the current usage is limited to this function only.
>


> > +
> > +    gst_init (NULL, NULL);
>
> Is this reentrant? If not, you need to serialize.
> 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?
>


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

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


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

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

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

>
> > +}
>
> --
> Rémi Denis-Courmont
> http://www.remlab.net/
>
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20140421/f9a1f9c9/attachment.html>


More information about the vlc-devel mailing list