[vlc-devel] [PATCH] New mpg123 audio converter

[Ludovic Fauvet]

On 09/02/2013 06:29 PM, Rémi Denis-Courmont wrote:
> Le lundi 2 septembre 2013 16:42:58 Ludovic Fauvet a écrit :
>> diff --git a/modules/audio_filter/Modules.am
>> b/modules/audio_filter/Modules.am index d3454e9..d730baf 100644
>> --- a/modules/audio_filter/Modules.am
>> +++ b/modules/audio_filter/Modules.am
>> @@ -51,6 +51,7 @@ SOURCES_a52tofloat32 = converter/a52tofloat32.c
>>  SOURCES_dtstospdif = converter/dtstospdif.c
>>  SOURCES_dtstofloat32 = converter/dtstofloat32.c
>>  SOURCES_mpgatofixed32 = converter/mpgatofixed32.c
>> +SOURCES_mpgatofloat32_mpg123 = converter/mpgatofloat32_mpg123.c
> 
> Can you please use a less awful plugin name?

I opted for mpg123.c since mpgatofixed32 should be renamed to
mpgatofloat32 since it outputs float32 now. The next obvious step would
then be to rename mpgatofixed32 to mad.

>>  libaudio_format_plugin_la_SOURCES = converter/format.c
>>  libaudio_format_plugin_la_CFLAGS = $(AM_CFLAGS)
>>  libaudio_format_plugin_la_LIBADD = $(AM_LIBADD) $(LIBM)
>> diff --git a/modules/audio_filter/converter/mpgatofloat32_mpg123.c
>> b/modules/audio_filter/converter/mpgatofloat32_mpg123.c new file mode
>> 100644
>> index 0000000..9588e5b
>> --- /dev/null
>> +++ b/modules/audio_filter/converter/mpgatofloat32_mpg123.c
>> @@ -0,0 +1,239 @@
>> +/**************************************************************************
>> *** + * mpgatofloat32_mpg123.c: MPEG-1 & 2 audio layer I, II, III + MPEG 2.5
>> + * decoder, using MPG123
>> +
>> ***************************************************************************
>> ** + * Copyright (C) 2001-2013 VLC authors and VideoLAN
>> + *
>> + * Authors: Christophe Massiot <massiot at via.ecp.fr>
>> + *          Jean-Paul Saman <jpsaman _at_ videolan _dot_ org>
>> + *          William Hahne <will07c5 at gmail.com>
>> + *          Ludovic Fauvet <etix at videolan.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU Lesser General Public License as published by
>> + * the Free Software Foundation; either version 2.1 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 Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser 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
>> +
>> ***************************************************************************
>> **/ +
>> +#include <mpg123.h>
>> +
>> +#ifdef HAVE_CONFIG_H
>> +# include "config.h"
>> +#endif
>> +#include <assert.h>
>> +
>> +#include <vlc_common.h>
>> +#include <vlc_plugin.h>
>> +#include <vlc_aout.h>
>> +#include <vlc_block.h>
>> +#include <vlc_filter.h>
>> +
>> +/**************************************************************************
>> *** + * Local prototypes
>> +
>> ***************************************************************************
>> **/ +static int  OpenFilter ( vlc_object_t * );
>> +static void CloseFilter( vlc_object_t * );
>> +static block_t *Convert( filter_t *, block_t * );
>> +
>> +/**************************************************************************
>> *** + * Local structures
>> +
>> ***************************************************************************
>> **/ +struct filter_sys_t
>> +{
>> +    mpg123_handle * p_handle;
>> +    unsigned char * p_buffer;
> 
> Why that?

Removed from the structure and changed the type to uint8_t*.

>> +
>> +    int             i_reject_count;
>> +    size_t          i_size;
> 
> Ditto.

Removed from the struct.

>> +    off_t           i_offset;
>> +};
>> +
>> +/**************************************************************************
>> *** + * Module descriptor
>> +
>> ***************************************************************************
>> **/ +vlc_module_begin ()
>> +    set_category( CAT_INPUT )
>> +    set_subcategory( SUBCAT_INPUT_ACODEC )
>> +    set_description( N_("MPEG audio decoder using mpg123") )
>> +    set_capability( "audio converter", 123 )
>> +    set_callbacks( OpenFilter, CloseFilter )
>> +vlc_module_end ()
>> +
>> +/**************************************************************************
>> *** + * DoWork: decode an MPEG audio frame.
>> +
>> ***************************************************************************
>> **/ +static void DoWork( filter_t * p_filter,
>> +                    block_t * p_in_buf, block_t * p_out_buf )
>> +{
>> +    filter_sys_t *p_sys = p_filter->p_sys;
>> +
>> +    p_out_buf->i_nb_samples = p_in_buf->i_nb_samples;
>> +    p_out_buf->i_buffer = p_in_buf->i_nb_samples * sizeof(float) *
>> +                          aout_FormatNbChannels( &p_filter->fmt_out.audio
>> ); +
>> +
>> +    /* Ignore the last 8 bytes due to a hack for libmad */
>> +    /* FIXME: This hack should be removed somehow --etix */
>> +    if ( p_in_buf->i_buffer < 8 )
>> +        return;
>> +    int i_err = mpg123_feed( p_sys->p_handle, p_in_buf->p_buffer,
>> +                                   p_in_buf->i_buffer - 8 );
>> +
>> +    /* Do the actual decoding now */
>> +    if ( i_err != MPG123_OK || mpg123_decode_frame( p_sys->p_handle,
>> +         &p_sys->i_offset, &p_sys->p_buffer, &p_sys->i_size ) != MPG123_OK
>> ) +    {
>> +        msg_Warn( p_filter, "mpg123 error: %s",
>> +            mpg123_strerror( p_sys->p_handle ) );
>> +        p_sys->i_reject_count = 3;
>> +    }
>> +    else if( p_in_buf->i_flags & BLOCK_FLAG_DISCONTINUITY )
>> +    {
>> +        p_sys->i_reject_count = 3;
>> +    }
>> +
>> +    if( p_sys->i_reject_count > 0 )
>> +    {
>> +        memset( p_out_buf->p_buffer, 0, p_out_buf->i_buffer );
>> +        p_sys->i_reject_count--;
>> +        return;
>> +    }
>> +
>> +    assert( p_out_buf->i_size >= p_sys->i_size );
>> +    memcpy( p_out_buf->p_buffer, p_sys->p_buffer, p_sys->i_size );
>> +}
>> +
>> +static block_t *Convert( filter_t *p_filter, block_t *p_block )
>> +{
>> +    if( !p_block || !p_block->i_nb_samples )
>> +    {
>> +        if( p_block )
>> +            block_Release( p_block );
>> +        return NULL;
>> +    }
>> +
>> +    size_t i_out_size = p_block->i_nb_samples *
>> +      p_filter->fmt_out.audio.i_bitspersample *
>> +      p_filter->fmt_out.audio.i_channels / 8;
>> +
>> +    block_t *p_out = block_Alloc( i_out_size );
>> +    if( unlikely( !p_out ) )
>> +    {
>> +        msg_Warn( p_filter, "can't get output buffer" );
> 
> Why?

Copy/pasted from mpgatofixed32 but makes no sense in case of low memory
so I removed it.

>> +        block_Release( p_block );
>> +        return NULL;
>> +    }
>> +
>> +    p_out->i_nb_samples = p_block->i_nb_samples;
>> +    p_out->i_dts = p_block->i_dts;
>> +    p_out->i_pts = p_block->i_pts;
>> +    p_out->i_length = p_block->i_length;
>> +
>> +    DoWork( p_filter, p_block, p_out );
>> +
>> +    block_Release( p_block );
>> +
>> +    return p_out;
>> +}
>> +
>> +/**************************************************************************
>> *** + * OpenFilter:
>> +
>> ***************************************************************************
>> **/ +static int OpenFilter( vlc_object_t *p_this )
>> +{
>> +    filter_t *p_filter = (filter_t *)p_this;
>> +    filter_sys_t *p_sys;
>> +
>> +    if( p_filter->fmt_in.i_codec != VLC_CODEC_MPGA &&
>> +        p_filter->fmt_in.i_codec != VLC_FOURCC('m','p','g','3') )
>> +        return VLC_EGENERIC;
>> +
>> +    if( !AOUT_FMTS_SIMILAR( &p_filter->fmt_in.audio,
>> &p_filter->fmt_out.audio ) ) +        return VLC_EGENERIC;
>> +
>> +    if( p_filter->fmt_out.audio.i_format != VLC_CODEC_FL32 )
>> +        return VLC_EGENERIC;
>> +
>> +    /* Allocate the memory needed to store the module's structure */
>> +    p_sys = p_filter->p_sys = malloc( sizeof(filter_sys_t) );
>> +    if( p_sys == NULL )
>> +        return VLC_EGENERIC;
>> +
>> +    /* Initialize libmpg123 */
>> +    if( mpg123_init() != MPG123_OK ||
> 
> Not reentrant.

I've added two new functions to Init and Exit the library in a reentrant
way. I used a global mutex + refcounter, I hope this is fine. I don't
really like adding more globals but this is the only way left to the
modules, right?

>> +      ( p_sys->p_handle = mpg123_new( NULL, NULL ) ) == NULL )
>> +    {
>> +        goto error;
>> +    }
>> +
>> +    /* Open a new bitstream */
>> +    if( mpg123_open_feed( p_sys->p_handle ) != MPG123_OK )
>> +    {
>> +        msg_Err( p_this, "mpg123 error: can't open feed" );
>> +        mpg123_delete( p_sys->p_handle );
>> +        goto error;
>> +    }
>> +
>> +    /* Disable resync stream after error */
>> +    mpg123_param( p_sys->p_handle, MPG123_ADD_FLAGS, MPG123_NO_RESYNC, 0 );
>> +
>> +    /* Setup output format */
>> +    mpg123_format_none( p_sys->p_handle );
>> +
>> +    if( MPG123_OK != mpg123_format( p_sys->p_handle,
>> +                                    p_filter->fmt_in.audio.i_rate,
>> +                                    MPG123_MONO | MPG123_STEREO,
>> +                                    MPG123_ENC_FLOAT_32 ) )
>> +    {
>> +        msg_Err( p_this, "mpg123 error: %s",
>> +                mpg123_strerror( p_sys->p_handle ) );
>> +        mpg123_close( p_sys->p_handle );
>> +        mpg123_delete( p_sys->p_handle );
>> +        goto error;
>> +    }
>> +
>> +    p_sys->i_reject_count = 0;
>> +    p_filter->pf_audio_filter = Convert;
>> +    p_filter->fmt_out.audio.i_format = p_filter->fmt_out.i_codec;
>> +    p_filter->fmt_out.audio.i_bitspersample =
>> +        aout_BitsPerSample( p_filter->fmt_out.i_codec );
>> +
>> +    msg_Dbg( p_this, "%4.4s->%4.4s, bits per sample: %i",
>> +             (char *)&p_filter->fmt_in.i_codec,
>> +             (char *)&p_filter->fmt_out.i_codec,
>> +             p_filter->fmt_in.audio.i_bitspersample );
>> +
>> +    return VLC_SUCCESS;
>> +error:
>> +    mpg123_exit();
>> +    free( p_sys );
>> +    return VLC_EGENERIC;
>> +}
>> +
>> +/**************************************************************************
>> *** + * CloseFilter : deallocate data structures
>> +
>> ***************************************************************************
>> **/ +static void CloseFilter( vlc_object_t *p_this )
>> +{
>> +    filter_t *p_filter = (filter_t *)p_this;
>> +    filter_sys_t *p_sys = p_filter->p_sys;
>> +
>> +    mpg123_close( p_sys->p_handle );
>> +    mpg123_delete( p_sys->p_handle );
>> +    mpg123_exit();
>> +    free( p_sys );
>> +}
>> +

Thanks Rémi.

-- 
Ludovic Fauvet
www.videolan.org