Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[David Fletcher]

On Fri May 6 21:29:51 EEST 2022 Lynne dev at lynne.ee wrote:

> I disagree with this patch. The native decoder should be fixed instead,
> or if it's not able to be fixed, users should use specialized libraries.
> We're not adding a yet another decoding library to fix an issue on
> very old ARM chips.

> I suspect the problem is with the MDCT. We already have a better and
> more precise fixed-point MDCT in libavutil/tx. The plan was to replace
> the MDCT in mp3 once lavu/tx has more assembly, but if it helps,
> should be fine to replace it now.

Hi Lynne,

I don't have a strong feeling either way. The patch (an earlier version)
is already doing what I needed in the application with the Reciva
radios. I posted it here to contribute back to FFmpeg, but if there's a
better way to fix the problem that's great.

If there's a better version of MDCT integrated into the native mp3 fixed
point decoder I can test that on the ARMv4 hardware to see if the
decoding bug is fixed. Certainly these are old chips but still lots of
them around - a big part of them being useful is the assembly
optimisations otherwise everything just slows down too much.

Best regards, David.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[Lynne]

5 May 2022, 01:47 by David@megapico.co.uk:

>> Andreas Rheinhardt wrote:
>>
>> David Fletcher:
>> > Following today's posts about help with submitting patches I realised I
>> > sent the libmad patch yesterday in the wrong format. Apologies, I was
>> > not familiar with the git format patches.
>> > 
>> > Hopefully the attached version is now in the correct format against the
>> > current master branch.
>> > 
>> > The bug report about why this exists is at the following link, including
>> > a link to sample distorted audio from decoding an mp3 stream on ARMv4
>> > hardware: https://trac.ffmpeg.org/ticket/9764
>> > 
>> > Best regards, David.
>> > 
>>

I disagree with this patch. The native decoder should be fixed instead,
or if it's not able to be fixed, users should use specialized libraries.
We're not adding a yet another decoding library to fix an issue on
very old ARM chips.

I suspect the problem is with the MDCT. We already have a better and
more precise fixed-point MDCT in libavutil/tx. The plan was to replace
the MDCT in mp3 once lavu/tx has more assembly, but if it helps,
should be fine to replace it now.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[David Fletcher]

[-- Attachment #1: Type: text/plain, Size: 10150 bytes --]

> Andreas Rheinhardt wrote:
> 
> David Fletcher:
> > Following today's posts about help with submitting patches I realised I
> > sent the libmad patch yesterday in the wrong format. Apologies, I was
> > not familiar with the git format patches.
> > 
> > Hopefully the attached version is now in the correct format against the
> > current master branch.
> > 
> > The bug report about why this exists is at the following link, including
> > a link to sample distorted audio from decoding an mp3 stream on ARMv4
> > hardware: https://trac.ffmpeg.org/ticket/9764
> > 
> > Best regards, David.
> > 
> 
> > 
> > diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> > index c47133aa18..e3df6178c8 100644
> > --- a/libavcodec/allcodecs.c
> > +++ b/libavcodec/allcodecs.c
> > @@ -744,6 +744,7 @@ extern const FFCodec ff_libcodec2_decoder;
> >  extern const FFCodec ff_libdav1d_decoder;
> >  extern const FFCodec ff_libdavs2_decoder;
> >  extern const FFCodec ff_libfdk_aac_encoder;
> > +extern const AVCodec ff_libmad_decoder;
> >  extern const FFCodec ff_libfdk_aac_decoder;
> >  extern const FFCodec ff_libgsm_encoder;
> >  extern const FFCodec ff_libgsm_decoder;
> 
> This should look weird to you.

Now you've pointed it out - yes, it does! This error was matched by a similar
use of AVCodec in place of FFCodec in the libmaddec.c file. It seems these
structures have enough in common that I'd got away with it running without
noticing this. 

> 
> > 
> > diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
> > index 8b317fa121..be70f4a71c 100644
> > --- a/libavcodec/codec_id.h
> > +++ b/libavcodec/codec_id.h
> > @@ -519,6 +519,7 @@ enum AVCodecID {
> >      AV_CODEC_ID_FASTAUDIO,
> >      AV_CODEC_ID_MSNSIREN,
> >      AV_CODEC_ID_DFPWM,
> > +    AV_CODEC_ID_LIBMAD,
> >  
> >      /* subtitle codecs */
> >      AV_CODEC_ID_FIRST_SUBTITLE = 0x17000,          ///< A dummy ID
> > pointing at the start of subtitle codecs.
> 
> This makes no sense: Your decoder is still expected to decode MP3 and
> not a new, previously unsupported format.

This was left over from some development experiments while I was working out
how to integrate libmad. It was not used and needed to be removed. The
ff_libmad_decoder structure in libmaddec.c already used AV_CODEC_ID_MP3 and
never mentioned AV_CODEC_ID_LIBMAD.


> 
> > diff --git a/libavcodec/libmaddec.c b/libavcodec/libmaddec.c
> > new file mode 100644
> > index 0000000000..7082c53f4d
> > --- /dev/null
> > +++ b/libavcodec/libmaddec.c
> > @@ -0,0 +1,181 @@
> > +/*
> > + * MP3 decoder using libmad
> > + * Copyright (c) 2022 David Fletcher
> > + *
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg 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.
> > + *
> > + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > 02110-1301 USA
> > + */
> > +
> > +#include <mad.h>
> > +
> > +#include "libavutil/channel_layout.h"
> > +#include "libavutil/common.h"
> > +#include "avcodec.h"
> > +#include "internal.h"
> > +#include "decode.h"
> > +
> > +#define MAD_BUFSIZE (32 * 1024)
> > +#define MIN(a, b) ((a) < (b) ? (a) : (b))
> > +
> > +typedef struct libmad_context {
> > +    uint8_t input_buffer[MAD_BUFSIZE+MAD_BUFFER_GUARD];
> > +    struct mad_synth  synth; 
> > +    struct mad_stream stream;
> > +    struct mad_frame  frame;
> > +    struct mad_header header;
> > +    int got_header;
> > +}libmad_context;		
> > +
> > +/* utility to scale and round samples to 16 bits */
> > +static inline signed int mad_scale(mad_fixed_t sample)
> > +{
> > +     /* round */
> > +     sample += (1L << (MAD_F_FRACBITS - 16));
> > + 
> > +     /* clip */
> > +     if (sample >= MAD_F_ONE)
> > +         sample = MAD_F_ONE - 1;
> > +     else if (sample < -MAD_F_ONE)
> > +         sample = -MAD_F_ONE;
> > +    
> > +     /* quantize */
> > +     return sample >> (MAD_F_FRACBITS + 1 - 16);
> > +}
> > +
> > +static av_cold int libmad_decode_init(AVCodecContext *avc)
> > +{
> > +     libmad_context *mad = avc->priv_data;
> > +
> > +     mad_synth_init  (&mad->synth);
> > +     mad_stream_init (&mad->stream);
> > +     mad_frame_init  (&mad->frame);
> > +     mad->got_header = 0;
> > +
> > +     return 0;
> > +}
> > +
> > +static av_cold int libmad_decode_close(AVCodecContext *avc)
> > +{
> > +     libmad_context *mad = avc->priv_data;
> > +
> > +     mad_synth_finish(&mad->synth);
> > +     mad_frame_finish(&mad->frame);
> > +     mad_stream_finish(&mad->stream);
> > +
> > +     mad = NULL;
> 
> This is pointless as the lifetime of this pointer ends with returning
> from this function anyway.

Thanks for pointing this out - removed now. 

> 
> > +    
> > +     return 0;
> > +}
> > +
> > +static int libmad_decode_frame(AVCodecContext *avc, void *data,
> > +                          int *got_frame_ptr, AVPacket *pkt)
> > +{
> > +     AVFrame *frame = data;
> > +     libmad_context *mad = avc->priv_data;
> > +     struct mad_pcm *pcm;
> > +     mad_fixed_t const *left_ch;
> > +     mad_fixed_t const *right_ch;
> > +     int16_t *output;
> > +     int nsamples;
> > +     int nchannels;
> > +     size_t bytes_read = 0;
> > +     size_t remaining = 0;
> > +     
> > +     if (!avc)
> > +	 return 0;
> 
> A codec can presume the AVCodecContext to not be NULL.
> 
> > +     
> > +     if (!mad)
> > +	 return 0;
> > +     
> 
> Similarly, every codec can presume the Codec's private data to be
> allocated (and be retained between calls to the same AVCodecContext
> instance).
> (Furthermore, the initialization of mad presumes avc to be not NULL,
> which makes the above check pointless.)

I'd played it safe by including checks on things - but since you've confirmed
that AVCodecContext will not be NULL and private data will always be
allocated these can be removed. 


> 
> > +     remaining = mad->stream.bufend - mad->stream.next_frame;
> > +     memmove(mad->input_buffer, mad->stream.next_frame, remaining);
> > +     bytes_read = MIN(pkt->size, MAD_BUFSIZE - remaining);
> > +     memcpy(mad->input_buffer+remaining, pkt->data, bytes_read);
> > +     
> > +     if (bytes_read == 0){
> > +	 *got_frame_ptr = 0;
> > +	 return 0;
> > +     }
> > +     
> > +     mad_stream_buffer(&mad->stream, mad->input_buffer, remaining +
> > bytes_read);
> > +     mad->stream.error = 0;
> > +     
> > +     if(!mad->got_header){
> > +	 mad_header_decode(&mad->header, &mad->stream);
> > +	 mad->got_header = 1;
> > +	 avc->frame_size = 32 * (mad->header.layer == MAD_LAYER_I ? 12 :
> > \
> > +				 ((mad->header.layer == MAD_LAYER_III &&
> > \
> > +				   (mad->header.flags &
> > MAD_FLAG_LSF_EXT)) ? 18 : 36));
> > +	 avc->sample_fmt = AV_SAMPLE_FMT_S16;
> > +	 if(mad->header.mode == MAD_MODE_SINGLE_CHANNEL){
> > +	     avc->channel_layout = AV_CH_LAYOUT_MONO;
> > +	     avc->channels = 1;
> > +	 }else{
> > +	     avc->channel_layout = AV_CH_LAYOUT_STEREO;
> > +	     avc->channels = 2;
> > +	 }
> > +     }
> > +     
> > +     frame->channel_layout = avc->channel_layout;
> > +     frame->format = avc->sample_fmt;
> > +     frame->channels = avc->channels;
> > +     frame->nb_samples = avc->frame_size; 
> > +     
> > +     if ((ff_get_buffer(avc, frame, 0)) < 0)
> > +	 return 0;
> 
> Return the error.

Modified to return the error now

> 
> > +     
> > +     if (mad_frame_decode(&mad->frame, &mad->stream) == -1) {
> > +	 *got_frame_ptr = 0;
> > +	 return mad->stream.bufend - mad->stream.next_frame;
> > +     }
> > +     
> > +     mad_synth_frame (&mad->synth, &mad->frame);
> > +     
> > +     pcm = &mad->synth.pcm;
> > +     output = (int16_t *)frame->data[0];
> > +     nsamples = pcm->length;
> > +     nchannels = pcm->channels;
> > +     left_ch = pcm->samples[0];
> > +     right_ch = pcm->samples[1];
> > +     while (nsamples--) {
> > +	 *output++ = mad_scale(*(left_ch++));
> > +	 if (nchannels == 2) {
> > +	     *output++ = mad_scale(*(right_ch++));
> > +	 }
> > +	 //Players should recognise mono and play through both channels
> > +	 //Writing the same thing to both left and right channels here
> > causes
> > +	 //memory issues as it creates double the number of samples
> > allocated.
> > +     }
> > +     
> > +     *got_frame_ptr = 1;
> > +     
> > +     return mad->stream.bufend - mad->stream.next_frame;
> > +}
> > +
> > +AVCodec ff_libmad_decoder = {
> > +    .name           = "libmad",
> > +    .long_name      = NULL_IF_CONFIG_SMALL("libmad MP3 decoder"),
> > +    .wrapper_name   = "libmad",
> > +    .type           = AVMEDIA_TYPE_AUDIO,
> > +    .id             = AV_CODEC_ID_MP3,
> > +    .sample_fmts    = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16,
> > AV_SAMPLE_FMT_NONE },
> > +    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_CHANNEL_CONF,
> > +    .priv_data_size = sizeof(libmad_context),
> > +    .init           = libmad_decode_init,
> > +    .close          = libmad_decode_close,
> > +    .decode         = libmad_decode_frame
> > +};
> 
> This should not even compile with latest master.
> 
> - Andreas

Hi Andreas,

Many thanks for your review and help to improve this. I've inserted
comments/replies above, and an updated version of the patch file is attached.
It compiles and runs, and I've just converted a range of mp3 files with it. 

I've realised I was suppressing some warnings when I compiled previously,
including about deprecated code in use of avc->channel_layout and
avc->channels. I've made updates to use the avc->ch_layout and
avc->ch_layout.nb_channels in the updated patch. Could you take another look?

Best regards, David.










[-- Attachment #2: libmad_v2-git-master.patch --]
[-- Type: application/octet-stream, Size: 9250 bytes --]

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[Andreas Rheinhardt]

David Fletcher:
> Following today's posts about help with submitting patches I realised I
> sent the libmad patch yesterday in the wrong format. Apologies, I was
> not familiar with the git format patches.
> 
> Hopefully the attached version is now in the correct format against the
> current master branch.
> 
> The bug report about why this exists is at the following link, including
> a link to sample distorted audio from decoding an mp3 stream on ARMv4
> hardware: https://trac.ffmpeg.org/ticket/9764
> 
> Best regards, David.
> 

> 
> diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
> index c47133aa18..e3df6178c8 100644
> --- a/libavcodec/allcodecs.c
> +++ b/libavcodec/allcodecs.c
> @@ -744,6 +744,7 @@ extern const FFCodec ff_libcodec2_decoder;
>  extern const FFCodec ff_libdav1d_decoder;
>  extern const FFCodec ff_libdavs2_decoder;
>  extern const FFCodec ff_libfdk_aac_encoder;
> +extern const AVCodec ff_libmad_decoder;
>  extern const FFCodec ff_libfdk_aac_decoder;
>  extern const FFCodec ff_libgsm_encoder;
>  extern const FFCodec ff_libgsm_decoder;

This should look weird to you.

> 
> diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
> index 8b317fa121..be70f4a71c 100644
> --- a/libavcodec/codec_id.h
> +++ b/libavcodec/codec_id.h
> @@ -519,6 +519,7 @@ enum AVCodecID {
>      AV_CODEC_ID_FASTAUDIO,
>      AV_CODEC_ID_MSNSIREN,
>      AV_CODEC_ID_DFPWM,
> +    AV_CODEC_ID_LIBMAD,
>  
>      /* subtitle codecs */
>      AV_CODEC_ID_FIRST_SUBTITLE = 0x17000,          ///< A dummy ID pointing at the start of subtitle codecs.

This makes no sense: Your decoder is still expected to decode MP3 and
not a new, previously unsupported format.

> diff --git a/libavcodec/libmaddec.c b/libavcodec/libmaddec.c
> new file mode 100644
> index 0000000000..7082c53f4d
> --- /dev/null
> +++ b/libavcodec/libmaddec.c
> @@ -0,0 +1,181 @@
> +/*
> + * MP3 decoder using libmad
> + * Copyright (c) 2022 David Fletcher
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg 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.
> + *
> + * FFmpeg 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 FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <mad.h>
> +
> +#include "libavutil/channel_layout.h"
> +#include "libavutil/common.h"
> +#include "avcodec.h"
> +#include "internal.h"
> +#include "decode.h"
> +
> +#define MAD_BUFSIZE (32 * 1024)
> +#define MIN(a, b) ((a) < (b) ? (a) : (b))
> +
> +typedef struct libmad_context {
> +    uint8_t input_buffer[MAD_BUFSIZE+MAD_BUFFER_GUARD];
> +    struct mad_synth  synth; 
> +    struct mad_stream stream;
> +    struct mad_frame  frame;
> +    struct mad_header header;
> +    int got_header;
> +}libmad_context;		
> +
> +/* utility to scale and round samples to 16 bits */
> +static inline signed int mad_scale(mad_fixed_t sample)
> +{
> +     /* round */
> +     sample += (1L << (MAD_F_FRACBITS - 16));
> + 
> +     /* clip */
> +     if (sample >= MAD_F_ONE)
> +         sample = MAD_F_ONE - 1;
> +     else if (sample < -MAD_F_ONE)
> +         sample = -MAD_F_ONE;
> +    
> +     /* quantize */
> +     return sample >> (MAD_F_FRACBITS + 1 - 16);
> +}
> +
> +static av_cold int libmad_decode_init(AVCodecContext *avc)
> +{
> +     libmad_context *mad = avc->priv_data;
> +
> +     mad_synth_init  (&mad->synth);
> +     mad_stream_init (&mad->stream);
> +     mad_frame_init  (&mad->frame);
> +     mad->got_header = 0;
> +
> +     return 0;
> +}
> +
> +static av_cold int libmad_decode_close(AVCodecContext *avc)
> +{
> +     libmad_context *mad = avc->priv_data;
> +
> +     mad_synth_finish(&mad->synth);
> +     mad_frame_finish(&mad->frame);
> +     mad_stream_finish(&mad->stream);
> +
> +     mad = NULL;

This is pointless as the lifetime of this pointer ends with returning
from this function anyway.

> +    
> +     return 0;
> +}
> +
> +static int libmad_decode_frame(AVCodecContext *avc, void *data,
> +                          int *got_frame_ptr, AVPacket *pkt)
> +{
> +     AVFrame *frame = data;
> +     libmad_context *mad = avc->priv_data;
> +     struct mad_pcm *pcm;
> +     mad_fixed_t const *left_ch;
> +     mad_fixed_t const *right_ch;
> +     int16_t *output;
> +     int nsamples;
> +     int nchannels;
> +     size_t bytes_read = 0;
> +     size_t remaining = 0;
> +     
> +     if (!avc)
> +	 return 0;

A codec can presume the AVCodecContext to not be NULL.

> +     
> +     if (!mad)
> +	 return 0;
> +     

Similarly, every codec can presume the Codec's private data to be
allocated (and be retained between calls to the same AVCodecContext
instance).
(Furthermore, the initialization of mad presumes avc to be not NULL,
which makes the above check pointless.)

> +     remaining = mad->stream.bufend - mad->stream.next_frame;
> +     memmove(mad->input_buffer, mad->stream.next_frame, remaining);
> +     bytes_read = MIN(pkt->size, MAD_BUFSIZE - remaining);
> +     memcpy(mad->input_buffer+remaining, pkt->data, bytes_read);
> +     
> +     if (bytes_read == 0){
> +	 *got_frame_ptr = 0;
> +	 return 0;
> +     }
> +     
> +     mad_stream_buffer(&mad->stream, mad->input_buffer, remaining + bytes_read);
> +     mad->stream.error = 0;
> +     
> +     if(!mad->got_header){
> +	 mad_header_decode(&mad->header, &mad->stream);
> +	 mad->got_header = 1;
> +	 avc->frame_size = 32 * (mad->header.layer == MAD_LAYER_I ? 12 : \
> +				 ((mad->header.layer == MAD_LAYER_III && \
> +				   (mad->header.flags & MAD_FLAG_LSF_EXT)) ? 18 : 36));
> +	 avc->sample_fmt = AV_SAMPLE_FMT_S16;
> +	 if(mad->header.mode == MAD_MODE_SINGLE_CHANNEL){
> +	     avc->channel_layout = AV_CH_LAYOUT_MONO;
> +	     avc->channels = 1;
> +	 }else{
> +	     avc->channel_layout = AV_CH_LAYOUT_STEREO;
> +	     avc->channels = 2;
> +	 }
> +     }
> +     
> +     frame->channel_layout = avc->channel_layout;
> +     frame->format = avc->sample_fmt;
> +     frame->channels = avc->channels;
> +     frame->nb_samples = avc->frame_size; 
> +     
> +     if ((ff_get_buffer(avc, frame, 0)) < 0)
> +	 return 0;

Return the error.

> +     
> +     if (mad_frame_decode(&mad->frame, &mad->stream) == -1) {
> +	 *got_frame_ptr = 0;
> +	 return mad->stream.bufend - mad->stream.next_frame;
> +     }
> +     
> +     mad_synth_frame (&mad->synth, &mad->frame);
> +     
> +     pcm = &mad->synth.pcm;
> +     output = (int16_t *)frame->data[0];
> +     nsamples = pcm->length;
> +     nchannels = pcm->channels;
> +     left_ch = pcm->samples[0];
> +     right_ch = pcm->samples[1];
> +     while (nsamples--) {
> +	 *output++ = mad_scale(*(left_ch++));
> +	 if (nchannels == 2) {
> +	     *output++ = mad_scale(*(right_ch++));
> +	 }
> +	 //Players should recognise mono and play through both channels
> +	 //Writing the same thing to both left and right channels here causes
> +	 //memory issues as it creates double the number of samples allocated.
> +     }
> +     
> +     *got_frame_ptr = 1;
> +     
> +     return mad->stream.bufend - mad->stream.next_frame;
> +}
> +
> +AVCodec ff_libmad_decoder = {
> +    .name           = "libmad",
> +    .long_name      = NULL_IF_CONFIG_SMALL("libmad MP3 decoder"),
> +    .wrapper_name   = "libmad",
> +    .type           = AVMEDIA_TYPE_AUDIO,
> +    .id             = AV_CODEC_ID_MP3,
> +    .sample_fmts    = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16, AV_SAMPLE_FMT_NONE },
> +    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_CHANNEL_CONF,
> +    .priv_data_size = sizeof(libmad_context),
> +    .init           = libmad_decode_init,
> +    .close          = libmad_decode_close,
> +    .decode         = libmad_decode_frame
> +};

This should not even compile with latest master.

- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[David Fletcher]

[-- Attachment #1: Type: text/plain, Size: 504 bytes --]

Following today's posts about help with submitting patches I realised I
sent the libmad patch yesterday in the wrong format. Apologies, I was
not familiar with the git format patches.

Hopefully the attached version is now in the correct format against the
current master branch.

The bug report about why this exists is at the following link, including
a link to sample distorted audio from decoding an mp3 stream on ARMv4
hardware: https://trac.ffmpeg.org/ticket/9764

Best regards, David.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: libmad-git-master.patch --]
[-- Type: text/x-patch; name="libmad-git-master.patch", Size: 9621 bytes --]

From c7012e7743d245bfc737bd921341a462cd5c843c Mon Sep 17 00:00:00 2001
From: David Fletcher <david@megapico.co.uk>
Date: Tue, 3 May 2022 19:20:23 +0100
Subject: [PATCH] Patched to work with libmad MP3 decoding codec

Signed-off-by: David Fletcher <david@megapico.co.uk>
---
 configure              |   4 +
 libavcodec/Makefile    |   1 +
 libavcodec/allcodecs.c |   1 +
 libavcodec/codec_id.h  |   1 +
 libavcodec/libmaddec.c | 181 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 188 insertions(+)
 create mode 100644 libavcodec/libmaddec.c

diff --git a/configure b/configure
index 196873c4aa..d5f5b79614 100755
--- a/configure
+++ b/configure
@@ -245,6 +245,7 @@ External library support:
   --enable-libklvanc       enable Kernel Labs VANC processing [no]
   --enable-libkvazaar      enable HEVC encoding via libkvazaar [no]
   --enable-liblensfun      enable lensfun lens correction [no]
+  --enable-libmad          enable MP3 decoding via libmad [no]
   --enable-libmodplug      enable ModPlug via libmodplug [no]
   --enable-libmp3lame      enable MP3 encoding via libmp3lame [no]
   --enable-libopencore-amrnb enable AMR-NB de/encoding via libopencore-amrnb [no]
@@ -1775,6 +1776,7 @@ EXTERNAL_LIBRARY_GPL_LIST="
     frei0r
     libcdio
     libdavs2
+    libmad
     librubberband
     libvidstab
     libx264
@@ -3339,6 +3341,7 @@ libjxl_decoder_deps="libjxl libjxl_threads"
 libjxl_encoder_deps="libjxl libjxl_threads"
 libkvazaar_encoder_deps="libkvazaar"
 libmodplug_demuxer_deps="libmodplug"
+libmad_decoder_deps="libmad"
 libmp3lame_encoder_deps="libmp3lame"
 libmp3lame_encoder_select="audio_frame_queue mpegaudioheader"
 libopencore_amrnb_decoder_deps="libopencore_amrnb"
@@ -6572,6 +6575,7 @@ if enabled libmfx; then
 fi
 
 enabled libmodplug        && require_pkg_config libmodplug libmodplug libmodplug/modplug.h ModPlug_Load
+enabled libmad            && require libmad "mad.h" mad_decoder_init -lmad
 enabled libmp3lame        && require "libmp3lame >= 3.98.3" lame/lame.h lame_set_VBR_quality -lmp3lame $libm_extralibs
 enabled libmysofa         && { check_pkg_config libmysofa libmysofa mysofa.h mysofa_neighborhood_init_withstepdefine ||
                                require libmysofa mysofa.h mysofa_neighborhood_init_withstepdefine -lmysofa $zlib_extralibs; }
diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index cfaa6f196a..1e434222d9 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -1065,6 +1065,7 @@ OBJS-$(CONFIG_LIBILBC_ENCODER)            += libilbc.o
 OBJS-$(CONFIG_LIBJXL_DECODER)             += libjxldec.o libjxl.o
 OBJS-$(CONFIG_LIBJXL_ENCODER)             += libjxlenc.o libjxl.o
 OBJS-$(CONFIG_LIBKVAZAAR_ENCODER)         += libkvazaar.o
+OBJS-$(CONFIG_LIBMAD_DECODER)             += libmaddec.o
 OBJS-$(CONFIG_LIBMP3LAME_ENCODER)         += libmp3lame.o
 OBJS-$(CONFIG_LIBOPENCORE_AMRNB_DECODER)  += libopencore-amr.o
 OBJS-$(CONFIG_LIBOPENCORE_AMRNB_ENCODER)  += libopencore-amr.o
diff --git a/libavcodec/allcodecs.c b/libavcodec/allcodecs.c
index c47133aa18..e3df6178c8 100644
--- a/libavcodec/allcodecs.c
+++ b/libavcodec/allcodecs.c
@@ -744,6 +744,7 @@ extern const FFCodec ff_libcodec2_decoder;
 extern const FFCodec ff_libdav1d_decoder;
 extern const FFCodec ff_libdavs2_decoder;
 extern const FFCodec ff_libfdk_aac_encoder;
+extern const AVCodec ff_libmad_decoder;
 extern const FFCodec ff_libfdk_aac_decoder;
 extern const FFCodec ff_libgsm_encoder;
 extern const FFCodec ff_libgsm_decoder;
diff --git a/libavcodec/codec_id.h b/libavcodec/codec_id.h
index 8b317fa121..be70f4a71c 100644
--- a/libavcodec/codec_id.h
+++ b/libavcodec/codec_id.h
@@ -519,6 +519,7 @@ enum AVCodecID {
     AV_CODEC_ID_FASTAUDIO,
     AV_CODEC_ID_MSNSIREN,
     AV_CODEC_ID_DFPWM,
+    AV_CODEC_ID_LIBMAD,
 
     /* subtitle codecs */
     AV_CODEC_ID_FIRST_SUBTITLE = 0x17000,          ///< A dummy ID pointing at the start of subtitle codecs.
diff --git a/libavcodec/libmaddec.c b/libavcodec/libmaddec.c
new file mode 100644
index 0000000000..7082c53f4d
--- /dev/null
+++ b/libavcodec/libmaddec.c
@@ -0,0 +1,181 @@
+/*
+ * MP3 decoder using libmad
+ * Copyright (c) 2022 David Fletcher
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <mad.h>
+
+#include "libavutil/channel_layout.h"
+#include "libavutil/common.h"
+#include "avcodec.h"
+#include "internal.h"
+#include "decode.h"
+
+#define MAD_BUFSIZE (32 * 1024)
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+
+typedef struct libmad_context {
+    uint8_t input_buffer[MAD_BUFSIZE+MAD_BUFFER_GUARD];
+    struct mad_synth  synth; 
+    struct mad_stream stream;
+    struct mad_frame  frame;
+    struct mad_header header;
+    int got_header;
+}libmad_context;		
+
+/* utility to scale and round samples to 16 bits */
+static inline signed int mad_scale(mad_fixed_t sample)
+{
+     /* round */
+     sample += (1L << (MAD_F_FRACBITS - 16));
+ 
+     /* clip */
+     if (sample >= MAD_F_ONE)
+         sample = MAD_F_ONE - 1;
+     else if (sample < -MAD_F_ONE)
+         sample = -MAD_F_ONE;
+    
+     /* quantize */
+     return sample >> (MAD_F_FRACBITS + 1 - 16);
+}
+
+static av_cold int libmad_decode_init(AVCodecContext *avc)
+{
+     libmad_context *mad = avc->priv_data;
+
+     mad_synth_init  (&mad->synth);
+     mad_stream_init (&mad->stream);
+     mad_frame_init  (&mad->frame);
+     mad->got_header = 0;
+
+     return 0;
+}
+
+static av_cold int libmad_decode_close(AVCodecContext *avc)
+{
+     libmad_context *mad = avc->priv_data;
+
+     mad_synth_finish(&mad->synth);
+     mad_frame_finish(&mad->frame);
+     mad_stream_finish(&mad->stream);
+
+     mad = NULL;
+    
+     return 0;
+}
+
+static int libmad_decode_frame(AVCodecContext *avc, void *data,
+                          int *got_frame_ptr, AVPacket *pkt)
+{
+     AVFrame *frame = data;
+     libmad_context *mad = avc->priv_data;
+     struct mad_pcm *pcm;
+     mad_fixed_t const *left_ch;
+     mad_fixed_t const *right_ch;
+     int16_t *output;
+     int nsamples;
+     int nchannels;
+     size_t bytes_read = 0;
+     size_t remaining = 0;
+     
+     if (!avc)
+	 return 0;
+     
+     if (!mad)
+	 return 0;
+     
+     remaining = mad->stream.bufend - mad->stream.next_frame;
+     memmove(mad->input_buffer, mad->stream.next_frame, remaining);
+     bytes_read = MIN(pkt->size, MAD_BUFSIZE - remaining);
+     memcpy(mad->input_buffer+remaining, pkt->data, bytes_read);
+     
+     if (bytes_read == 0){
+	 *got_frame_ptr = 0;
+	 return 0;
+     }
+     
+     mad_stream_buffer(&mad->stream, mad->input_buffer, remaining + bytes_read);
+     mad->stream.error = 0;
+     
+     if(!mad->got_header){
+	 mad_header_decode(&mad->header, &mad->stream);
+	 mad->got_header = 1;
+	 avc->frame_size = 32 * (mad->header.layer == MAD_LAYER_I ? 12 : \
+				 ((mad->header.layer == MAD_LAYER_III && \
+				   (mad->header.flags & MAD_FLAG_LSF_EXT)) ? 18 : 36));
+	 avc->sample_fmt = AV_SAMPLE_FMT_S16;
+	 if(mad->header.mode == MAD_MODE_SINGLE_CHANNEL){
+	     avc->channel_layout = AV_CH_LAYOUT_MONO;
+	     avc->channels = 1;
+	 }else{
+	     avc->channel_layout = AV_CH_LAYOUT_STEREO;
+	     avc->channels = 2;
+	 }
+     }
+     
+     frame->channel_layout = avc->channel_layout;
+     frame->format = avc->sample_fmt;
+     frame->channels = avc->channels;
+     frame->nb_samples = avc->frame_size; 
+     
+     if ((ff_get_buffer(avc, frame, 0)) < 0)
+	 return 0;
+     
+     if (mad_frame_decode(&mad->frame, &mad->stream) == -1) {
+	 *got_frame_ptr = 0;
+	 return mad->stream.bufend - mad->stream.next_frame;
+     }
+     
+     mad_synth_frame (&mad->synth, &mad->frame);
+     
+     pcm = &mad->synth.pcm;
+     output = (int16_t *)frame->data[0];
+     nsamples = pcm->length;
+     nchannels = pcm->channels;
+     left_ch = pcm->samples[0];
+     right_ch = pcm->samples[1];
+     while (nsamples--) {
+	 *output++ = mad_scale(*(left_ch++));
+	 if (nchannels == 2) {
+	     *output++ = mad_scale(*(right_ch++));
+	 }
+	 //Players should recognise mono and play through both channels
+	 //Writing the same thing to both left and right channels here causes
+	 //memory issues as it creates double the number of samples allocated.
+     }
+     
+     *got_frame_ptr = 1;
+     
+     return mad->stream.bufend - mad->stream.next_frame;
+}
+
+AVCodec ff_libmad_decoder = {
+    .name           = "libmad",
+    .long_name      = NULL_IF_CONFIG_SMALL("libmad MP3 decoder"),
+    .wrapper_name   = "libmad",
+    .type           = AVMEDIA_TYPE_AUDIO,
+    .id             = AV_CODEC_ID_MP3,
+    .sample_fmts    = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16, AV_SAMPLE_FMT_NONE },
+    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_CHANNEL_CONF,
+    .priv_data_size = sizeof(libmad_context),
+    .init           = libmad_decode_init,
+    .close          = libmad_decode_close,
+    .decode         = libmad_decode_frame
+};
+
-- 
2.17.6


[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[David Fletcher]

On 2/5/2022, "Martin Storsjö" martin at martin.st  wrote:
> That sounds like there's an issue with some of the ARM implementations of 
> DSP functions, that only exists in implementations for less featureful ARM 
> instruction sets.
> 
> (It could theoretically be a compiler bug also but I think it's much more 
> probable that it's a bug in a seldom used codepath in ffmpeg.)
> 
> If you pass "-cpuflags 0" to ffmpeg, does the issue vanish? (If it does, 
> it would suggest an issue in e.g. an armv5/armv6 specific implementation 
> of some DSP functions.) Does if vanish if you configure with 
> --arch=generic? (That would omit all ARM specific DSP routines and just 
> use plain C code.)
> 
> On a more modern ARM cpu, if it works correctly normally, does the issue 
> appear if you use "-cpuflags 0"? That would indicate an issue in one of 
> the ARM specific routines that are enabled without cpuflags (e.g. that 
> don't require anything above armv4).
> 

Hi Martin,

Thanks for these tips. I've posted a bug tracker about this issue, and
also created some sample audio files that demonstrate things working
well on ARMv7, but with distortion on ARMv4.
https://trac.ffmpeg.org/ticket/9764

So far I've been using the libav libraries rather than ffmpeg so it's
not so easy to pass "-cpuflags 0". But this is I think doing the same
thing I've been doing with when configuring to compile FFmpeg. I tried
a range of  --disable-vfp --disable-neon --enable-asm or --disable-asm.
I could not find a combination that makes the issue go away. As you say,
that could well indicate something in an ARM routine that always runs,
and is not superseded by an ARM5 or higher alternative when the CPU can
handle that.

I've not tried --arch=generic. I'm cross-compiling on x86 targeting
ARMv4 so the cross-compile will stop working properly if I take out
--arch=armv4t.

I'll do some further investigation and see if I can create ffmpeg
through the cross-compilation rather than the transcode program I've
been using (this is based on the example transcode_aac).

Many thanks, David.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[Martin Storsjö]

On Mon, 2 May 2022, David Fletcher wrote:

> On 2/5/2022, "Nicolas George" <george@nsup.org> wrote:
>> Is there a trac ticket? If not, please fill one: we would not want to
>> keep that bug.
>>
>> Regards,
>>
>> --
>>  Nicolas George
>
> Hi Nicolas,
>
> I'll prepare a test case to demonstrate the issue and fill in a ticket.
> As far as I can tell this is an issue affecting older ARM hardware, it
> cannot be reproduced on x86 hardware. I now have some additional ARM
> hardware with a more capable CPU which should help pin down the problem.

That sounds like there's an issue with some of the ARM implementations of 
DSP functions, that only exists in implementations for less featureful ARM 
instruction sets.

(It could theoretically be a compiler bug also but I think it's much more 
probable that it's a bug in a seldom used codepath in ffmpeg.)

If you pass "-cpuflags 0" to ffmpeg, does the issue vanish? (If it does, 
it would suggest an issue in e.g. an armv5/armv6 specific implementation 
of some DSP functions.) Does if vanish if you configure with 
--arch=generic? (That would omit all ARM specific DSP routines and just 
use plain C code.)

On a more modern ARM cpu, if it works correctly normally, does the issue 
appear if you use "-cpuflags 0"? That would indicate an issue in one of 
the ARM specific routines that are enabled without cpuflags (e.g. that 
don't require anything above armv4).

// Martin

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[Neal Gompa]

On Mon, May 2, 2022 at 1:42 PM David Fletcher <David@megapico.co.uk> wrote:
>
> On 2/5/2022, "Paul B Mahol" <onemda@gmail.com> wrote:
> >Only patches for master are accepted.
> >
> >No more wrappers.
> >
> >Thanks.
>
> Hi Paul,
>
> Please find attached the libmad MP3 decoding patch against the master
> (ffmpeg-master-b67572c).
>
> I'm not sure what "No more wrappers" means?
>

Don't worry about it. He doesn't like seeing the use of codec libraries.

That said, it's concerning that the mp3 codec is so broken in ffmpeg.
It would be good to have a bug report so someone can look at fixing
it.



--
真実はいつも一つ！/ Always, there's only one truth!
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[Timo Rothenpieler]

On 02.05.2022 19:45, David Fletcher wrote:
> On 2/5/2022, "Nicolas George" <george@nsup.org> wrote:
>> Is there a trac ticket? If not, please fill one: we would not want to
>> keep that bug.
>>
>> Regards,
>>
>> -- 
>>   Nicolas George
> 
> Hi Nicolas,
> 
> I'll prepare a test case to demonstrate the issue and fill in a ticket.
> As far as I can tell this is an issue affecting older ARM hardware, it
> cannot be reproduced on x86 hardware. I now have some additional ARM
> hardware with a more capable CPU which should help pin down the problem.
> 
> Best regards, David.

That sounds like a pretty serious issue to me, and I'd rather see that 
fixed than an entire other decoder library pulled in, just to work 
around it.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[David Fletcher]

On 2/5/2022, "Nicolas George" <george@nsup.org> wrote:
>Is there a trac ticket? If not, please fill one: we would not want to
>keep that bug.
>
>Regards,
>
>-- 
>  Nicolas George

Hi Nicolas,

I'll prepare a test case to demonstrate the issue and fill in a ticket.
As far as I can tell this is an issue affecting older ARM hardware, it
cannot be reproduced on x86 hardware. I now have some additional ARM
hardware with a more capable CPU which should help pin down the problem.

Best regards, David.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[David Fletcher]

[-- Attachment #1: Type: text/plain, Size: 313 bytes --]

On 2/5/2022, "Paul B Mahol" <onemda@gmail.com> wrote:
>Only patches for master are accepted.
>
>No more wrappers.
>
>Thanks.

Hi Paul,

Please find attached the libmad MP3 decoding patch against the master
(ffmpeg-master-b67572c).

I'm not sure what "No more wrappers" means?

Best regards, David.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: libmad-b67572c.patch --]
[-- Type: text/x-patch; name="libmad-b67572c.patch", Size: 9504 bytes --]

diff -Nur ./ffmpeg-master-b67572c/configure ./ffmpeg-master-b67572c-libmad/configure
--- ./ffmpeg-master-b67572c/configure	2022-05-02 16:10:22.000000000 +0100
+++ ./ffmpeg-master-b67572c-libmad/configure	2022-05-02 17:56:30.772026904 +0100
@@ -245,6 +245,7 @@
   --enable-libklvanc       enable Kernel Labs VANC processing [no]
   --enable-libkvazaar      enable HEVC encoding via libkvazaar [no]
   --enable-liblensfun      enable lensfun lens correction [no]
+  --enable-libmad          enable MP3 decoding via libmad [no]
   --enable-libmodplug      enable ModPlug via libmodplug [no]
   --enable-libmp3lame      enable MP3 encoding via libmp3lame [no]
   --enable-libopencore-amrnb enable AMR-NB de/encoding via libopencore-amrnb [no]
@@ -1775,6 +1776,7 @@
     frei0r
     libcdio
     libdavs2
+    libmad
     librubberband
     libvidstab
     libx264
@@ -3339,6 +3341,7 @@
 libjxl_encoder_deps="libjxl libjxl_threads"
 libkvazaar_encoder_deps="libkvazaar"
 libmodplug_demuxer_deps="libmodplug"
+libmad_decoder_deps="libmad"
 libmp3lame_encoder_deps="libmp3lame"
 libmp3lame_encoder_select="audio_frame_queue mpegaudioheader"
 libopencore_amrnb_decoder_deps="libopencore_amrnb"
@@ -6572,6 +6575,7 @@
 fi
 
 enabled libmodplug        && require_pkg_config libmodplug libmodplug libmodplug/modplug.h ModPlug_Load
+enabled libmad            && require libmad "mad.h" mad_decoder_init -lmad
 enabled libmp3lame        && require "libmp3lame >= 3.98.3" lame/lame.h lame_set_VBR_quality -lmp3lame $libm_extralibs
 enabled libmysofa         && { check_pkg_config libmysofa libmysofa mysofa.h mysofa_neighborhood_init_withstepdefine ||
                                require libmysofa mysofa.h mysofa_neighborhood_init_withstepdefine -lmysofa $zlib_extralibs; }
diff -Nur ./ffmpeg-master-b67572c/libavcodec/Makefile ./ffmpeg-master-b67572c-libmad/libavcodec/Makefile
--- ./ffmpeg-master-b67572c/libavcodec/Makefile	2022-05-02 16:10:22.000000000 +0100
+++ ./ffmpeg-master-b67572c-libmad/libavcodec/Makefile	2022-05-02 17:56:30.772026904 +0100
@@ -1065,6 +1065,7 @@
 OBJS-$(CONFIG_LIBJXL_DECODER)             += libjxldec.o libjxl.o
 OBJS-$(CONFIG_LIBJXL_ENCODER)             += libjxlenc.o libjxl.o
 OBJS-$(CONFIG_LIBKVAZAAR_ENCODER)         += libkvazaar.o
+OBJS-$(CONFIG_LIBMAD_DECODER)             += libmaddec.o
 OBJS-$(CONFIG_LIBMP3LAME_ENCODER)         += libmp3lame.o
 OBJS-$(CONFIG_LIBOPENCORE_AMRNB_DECODER)  += libopencore-amr.o
 OBJS-$(CONFIG_LIBOPENCORE_AMRNB_ENCODER)  += libopencore-amr.o
diff -Nur ./ffmpeg-master-b67572c/libavcodec/allcodecs.c ./ffmpeg-master-b67572c-libmad/libavcodec/allcodecs.c
--- ./ffmpeg-master-b67572c/libavcodec/allcodecs.c	2022-05-02 16:10:22.000000000 +0100
+++ ./ffmpeg-master-b67572c-libmad/libavcodec/allcodecs.c	2022-05-02 17:56:30.773026904 +0100
@@ -744,6 +744,7 @@
 extern const FFCodec ff_libdav1d_decoder;
 extern const FFCodec ff_libdavs2_decoder;
 extern const FFCodec ff_libfdk_aac_encoder;
+extern const AVCodec ff_libmad_decoder;
 extern const FFCodec ff_libfdk_aac_decoder;
 extern const FFCodec ff_libgsm_encoder;
 extern const FFCodec ff_libgsm_decoder;
diff -Nur ./ffmpeg-master-b67572c/libavcodec/codec_id.h ./ffmpeg-master-b67572c-libmad/libavcodec/codec_id.h
--- ./ffmpeg-master-b67572c/libavcodec/codec_id.h	2022-05-02 16:10:22.000000000 +0100
+++ ./ffmpeg-master-b67572c-libmad/libavcodec/codec_id.h	2022-05-02 18:01:05.995035711 +0100
@@ -519,6 +519,7 @@
     AV_CODEC_ID_FASTAUDIO,
     AV_CODEC_ID_MSNSIREN,
     AV_CODEC_ID_DFPWM,
+    AV_CODEC_ID_LIBMAD,
 
     /* subtitle codecs */
     AV_CODEC_ID_FIRST_SUBTITLE = 0x17000,          ///< A dummy ID pointing at the start of subtitle codecs.
diff -Nur ./ffmpeg-master-b67572c/libavcodec/libmaddec.c ./ffmpeg-master-b67572c-libmad/libavcodec/libmaddec.c
--- ./ffmpeg-master-b67572c/libavcodec/libmaddec.c	1970-01-01 01:00:00.000000000 +0100
+++ ./ffmpeg-master-b67572c-libmad/libavcodec/libmaddec.c	2022-05-02 18:11:33.062055778 +0100
@@ -0,0 +1,181 @@
+/*
+ * MP3 decoder using libmad
+ * Copyright (c) 2022 David Fletcher
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <mad.h>
+
+#include "libavutil/channel_layout.h"
+#include "libavutil/common.h"
+#include "avcodec.h"
+#include "internal.h"
+#include "decode.h"
+
+#define MAD_BUFSIZE (32 * 1024)
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+
+typedef struct libmad_context {
+    uint8_t input_buffer[MAD_BUFSIZE+MAD_BUFFER_GUARD];
+    struct mad_synth  synth; 
+    struct mad_stream stream;
+    struct mad_frame  frame;
+    struct mad_header header;
+    int got_header;
+}libmad_context;		
+
+/* utility to scale and round samples to 16 bits */
+static inline signed int mad_scale(mad_fixed_t sample)
+{
+     /* round */
+     sample += (1L << (MAD_F_FRACBITS - 16));
+ 
+     /* clip */
+     if (sample >= MAD_F_ONE)
+         sample = MAD_F_ONE - 1;
+     else if (sample < -MAD_F_ONE)
+         sample = -MAD_F_ONE;
+    
+     /* quantize */
+     return sample >> (MAD_F_FRACBITS + 1 - 16);
+}
+
+static av_cold int libmad_decode_init(AVCodecContext *avc)
+{
+     libmad_context *mad = avc->priv_data;
+
+     mad_synth_init  (&mad->synth);
+     mad_stream_init (&mad->stream);
+     mad_frame_init  (&mad->frame);
+     mad->got_header = 0;
+
+     return 0;
+}
+
+static av_cold int libmad_decode_close(AVCodecContext *avc)
+{
+     libmad_context *mad = avc->priv_data;
+
+     mad_synth_finish(&mad->synth);
+     mad_frame_finish(&mad->frame);
+     mad_stream_finish(&mad->stream);
+
+     mad = NULL;
+    
+     return 0;
+}
+
+static int libmad_decode_frame(AVCodecContext *avc, void *data,
+                          int *got_frame_ptr, AVPacket *pkt)
+{
+     AVFrame *frame = data;
+     libmad_context *mad = avc->priv_data;
+     struct mad_pcm *pcm;
+     mad_fixed_t const *left_ch;
+     mad_fixed_t const *right_ch;
+     int16_t *output;
+     int nsamples;
+     int nchannels;
+     size_t bytes_read = 0;
+     size_t remaining = 0;
+     
+     if (!avc)
+	 return 0;
+     
+     if (!mad)
+	 return 0;
+     
+     remaining = mad->stream.bufend - mad->stream.next_frame;
+     memmove(mad->input_buffer, mad->stream.next_frame, remaining);
+     bytes_read = MIN(pkt->size, MAD_BUFSIZE - remaining);
+     memcpy(mad->input_buffer+remaining, pkt->data, bytes_read);
+     
+     if (bytes_read == 0){
+	 *got_frame_ptr = 0;
+	 return 0;
+     }
+     
+     mad_stream_buffer(&mad->stream, mad->input_buffer, remaining + bytes_read);
+     mad->stream.error = 0;
+     
+     if(!mad->got_header){
+	 mad_header_decode(&mad->header, &mad->stream);
+	 mad->got_header = 1;
+	 avc->frame_size = 32 * (mad->header.layer == MAD_LAYER_I ? 12 : \
+				 ((mad->header.layer == MAD_LAYER_III && \
+				   (mad->header.flags & MAD_FLAG_LSF_EXT)) ? 18 : 36));
+	 avc->sample_fmt = AV_SAMPLE_FMT_S16;
+	 if(mad->header.mode == MAD_MODE_SINGLE_CHANNEL){
+	     avc->channel_layout = AV_CH_LAYOUT_MONO;
+	     avc->channels = 1;
+	 }else{
+	     avc->channel_layout = AV_CH_LAYOUT_STEREO;
+	     avc->channels = 2;
+	 }
+     }
+     
+     frame->channel_layout = avc->channel_layout;
+     frame->format = avc->sample_fmt;
+     frame->channels = avc->channels;
+     frame->nb_samples = avc->frame_size; 
+     
+     if ((ff_get_buffer(avc, frame, 0)) < 0)
+	 return 0;
+     
+     if (mad_frame_decode(&mad->frame, &mad->stream) == -1) {
+	 *got_frame_ptr = 0;
+	 return mad->stream.bufend - mad->stream.next_frame;
+     }
+     
+     mad_synth_frame (&mad->synth, &mad->frame);
+     
+     pcm = &mad->synth.pcm;
+     output = (int16_t *)frame->data[0];
+     nsamples = pcm->length;
+     nchannels = pcm->channels;
+     left_ch = pcm->samples[0];
+     right_ch = pcm->samples[1];
+     while (nsamples--) {
+	 *output++ = mad_scale(*(left_ch++));
+	 if (nchannels == 2) {
+	     *output++ = mad_scale(*(right_ch++));
+	 }
+	 //Players should recognise mono and play through both channels
+	 //Writing the same thing to both left and right channels here causes
+	 //memory issues as it creates double the number of samples allocated.
+     }
+     
+     *got_frame_ptr = 1;
+     
+     return mad->stream.bufend - mad->stream.next_frame;
+}
+
+AVCodec ff_libmad_decoder = {
+    .name           = "libmad",
+    .long_name      = NULL_IF_CONFIG_SMALL("libmad MP3 decoder"),
+    .wrapper_name   = "libmad",
+    .type           = AVMEDIA_TYPE_AUDIO,
+    .id             = AV_CODEC_ID_MP3,
+    .sample_fmts    = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16, AV_SAMPLE_FMT_NONE },
+    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_CHANNEL_CONF,
+    .priv_data_size = sizeof(libmad_context),
+    .init           = libmad_decode_init,
+    .close          = libmad_decode_close,
+    .decode         = libmad_decode_frame
+};
+

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[Nicolas George]

[-- Attachment #1.1: Type: text/plain, Size: 333 bytes --]

David Fletcher (12022-05-01):
> I hope that this is useful. Some context - I found the FFmpeg inbuild
> fixed point MP3 decoder produced very distorted audio for low bitrate
> streams (56k/s and lower).

Is there a trac ticket? If not, please fill one: we would not want to
keep that bug.

Regards,

-- 
  Nicolas George

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] PATCH - libmad MP3 decoding support

[Paul B Mahol]

Only patches for master are accepted.

No more wrappers.

Thanks.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] PATCH - libmad MP3 decoding support

[David Fletcher]

[-- Attachment #1: Type: text/plain, Size: 773 bytes --]

Please find attached a patch adding support for MP3 decoding using
libmad. This is against release ffmpeg-5.0.1, and works for libmad
0.15.1b (the most recent available).

I hope that this is useful. Some context - I found the FFmpeg inbuild
fixed point MP3 decoder produced very distorted audio for low bitrate
streams (56k/s and lower). This was on an ARMv4 CPU for which I needed
to make some adaptation to the fixed point maths routines which
currently target more recent CPUs. I was not able to resolve the
distortion issue and it was easier to integrate libmad decoding which is
already optimised for very fast decoding on the ARMv4 CPU. More info
about this application here:
http://www.megapico.co.uk/sharpfin/mediaserver.html.

Best regards, David.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: libmad.patch --]
[-- Type: text/x-patch; name="libmad.patch", Size: 9107 bytes --]

diff -Nur ./ffmpeg-5.0.1/configure ./ffmpeg-5.0.1-mad/configure
--- ./ffmpeg-5.0.1/configure	2022-04-04 15:40:22.000000000 +0100
+++ ./ffmpeg-5.0.1-mad/configure	2022-05-01 22:50:01.435432431 +0100
@@ -244,6 +244,7 @@
   --enable-libklvanc       enable Kernel Labs VANC processing [no]
   --enable-libkvazaar      enable HEVC encoding via libkvazaar [no]
   --enable-liblensfun      enable lensfun lens correction [no]
+  --enable-libmad          enable MP3 decoding via libmad [no]
   --enable-libmodplug      enable ModPlug via libmodplug [no]
   --enable-libmp3lame      enable MP3 encoding via libmp3lame [no]
   --enable-libopencore-amrnb enable AMR-NB de/encoding via libopencore-amrnb [no]
@@ -1772,6 +1773,7 @@
     frei0r
     libcdio
     libdavs2
+    libmad
     librubberband
     libvidstab
     libx264
@@ -3334,6 +3336,7 @@
 libilbc_encoder_deps="libilbc"
 libkvazaar_encoder_deps="libkvazaar"
 libmodplug_demuxer_deps="libmodplug"
+libmad_decoder_deps="libmad"
 libmp3lame_encoder_deps="libmp3lame"
 libmp3lame_encoder_select="audio_frame_queue mpegaudioheader"
 libopencore_amrnb_decoder_deps="libopencore_amrnb"
@@ -6569,6 +6572,7 @@
 fi
 
 enabled libmodplug        && require_pkg_config libmodplug libmodplug libmodplug/modplug.h ModPlug_Load
+enabled libmad            && require libmad "mad.h" mad_decoder_init -lmad
 enabled libmp3lame        && require "libmp3lame >= 3.98.3" lame/lame.h lame_set_VBR_quality -lmp3lame $libm_extralibs
 enabled libmysofa         && { check_pkg_config libmysofa libmysofa mysofa.h mysofa_neighborhood_init_withstepdefine ||
                                require libmysofa mysofa.h mysofa_neighborhood_init_withstepdefine -lmysofa $zlib_extralibs; }
diff -Nur ./ffmpeg-5.0.1/libavcodec/Makefile ./ffmpeg-5.0.1-mad/libavcodec/Makefile
--- ./ffmpeg-5.0.1/libavcodec/Makefile	2022-01-14 18:45:39.000000000 +0000
+++ ./ffmpeg-5.0.1-mad/libavcodec/Makefile	2022-04-06 22:56:06.000000000 +0100
@@ -1054,6 +1054,7 @@
 OBJS-$(CONFIG_LIBILBC_DECODER)            += libilbc.o
 OBJS-$(CONFIG_LIBILBC_ENCODER)            += libilbc.o
 OBJS-$(CONFIG_LIBKVAZAAR_ENCODER)         += libkvazaar.o
+OBJS-$(CONFIG_LIBMAD_DECODER)             += libmaddec.o
 OBJS-$(CONFIG_LIBMP3LAME_ENCODER)         += libmp3lame.o
 OBJS-$(CONFIG_LIBOPENCORE_AMRNB_DECODER)  += libopencore-amr.o
 OBJS-$(CONFIG_LIBOPENCORE_AMRNB_ENCODER)  += libopencore-amr.o
diff -Nur ./ffmpeg-5.0.1/libavcodec/allcodecs.c ./ffmpeg-5.0.1-mad/libavcodec/allcodecs.c
--- ./ffmpeg-5.0.1/libavcodec/allcodecs.c	2022-01-14 18:45:39.000000000 +0000
+++ ./ffmpeg-5.0.1-mad/libavcodec/allcodecs.c	2022-04-06 22:54:45.000000000 +0100
@@ -744,6 +744,7 @@
 extern const AVCodec ff_libgsm_ms_decoder;
 extern const AVCodec ff_libilbc_encoder;
 extern const AVCodec ff_libilbc_decoder;
+extern const AVCodec ff_libmad_decoder;
 extern const AVCodec ff_libmp3lame_encoder;
 extern const AVCodec ff_libopencore_amrnb_encoder;
 extern const AVCodec ff_libopencore_amrnb_decoder;
diff -Nur ./ffmpeg-5.0.1/libavcodec/codec_id.h ./ffmpeg-5.0.1-mad/libavcodec/codec_id.h
--- ./ffmpeg-5.0.1/libavcodec/codec_id.h	2022-01-14 18:45:39.000000000 +0000
+++ ./ffmpeg-5.0.1-mad/libavcodec/codec_id.h	2022-04-06 22:52:18.000000000 +0100
@@ -516,6 +516,7 @@
     AV_CODEC_ID_HCA,
     AV_CODEC_ID_FASTAUDIO,
     AV_CODEC_ID_MSNSIREN,
+    AV_CODEC_ID_LIBMAD,
 
     /* subtitle codecs */
     AV_CODEC_ID_FIRST_SUBTITLE = 0x17000,          ///< A dummy ID pointing at the start of subtitle codecs.
diff -Nur ./ffmpeg-5.0.1/libavcodec/libmaddec.c ./ffmpeg-5.0.1-mad/libavcodec/libmaddec.c
--- ./ffmpeg-5.0.1/libavcodec/libmaddec.c	1970-01-01 01:00:00.000000000 +0100
+++ ./ffmpeg-5.0.1-mad/libavcodec/libmaddec.c	2022-04-12 19:00:21.000000000 +0100
@@ -0,0 +1,181 @@
+/*
+ * MP3 decoder using libmad
+ * Copyright (c) 2022 David Fletcher
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg 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.
+ *
+ * FFmpeg 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 FFmpeg; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <mad.h>
+
+#include "libavutil/channel_layout.h"
+#include "libavutil/common.h"
+#include "avcodec.h"
+#include "internal.h"
+#include "decode.h"
+
+#define MAD_BUFSIZE (32 * 1024)
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+
+typedef struct libmad_context {
+  uint8_t input_buffer[MAD_BUFSIZE+MAD_BUFFER_GUARD];
+  struct mad_synth  synth; 
+  struct mad_stream stream;
+  struct mad_frame  frame;
+  struct mad_header header;
+  int got_header;
+}libmad_context;		
+
+/* utility to scale and round samples to 16 bits */
+static inline signed int mad_scale(mad_fixed_t sample)
+{
+  /* round */
+  sample += (1L << (MAD_F_FRACBITS - 16));
+  
+  /* clip */
+  if (sample >= MAD_F_ONE)
+    sample = MAD_F_ONE - 1;
+  else if (sample < -MAD_F_ONE)
+    sample = -MAD_F_ONE;
+  
+  /* quantize */
+  return sample >> (MAD_F_FRACBITS + 1 - 16);
+}
+
+static av_cold int libmad_decode_init(AVCodecContext *avc)
+{
+    libmad_context *mad = avc->priv_data;
+
+    mad_synth_init  (&mad->synth);
+    mad_stream_init (&mad->stream);
+    mad_frame_init  (&mad->frame);
+    mad->got_header = 0;
+
+    return 0;
+}
+
+static av_cold int libmad_decode_close(AVCodecContext *avc)
+{
+    libmad_context *mad = avc->priv_data;
+
+    mad_synth_finish(&mad->synth);
+    mad_frame_finish(&mad->frame);
+    mad_stream_finish(&mad->stream);
+
+    mad = NULL;
+    
+    return 0;
+}
+
+static int libmad_decode_frame(AVCodecContext *avc, void *data,
+                          int *got_frame_ptr, AVPacket *pkt)
+{
+  AVFrame *frame = data;
+  libmad_context *mad = avc->priv_data;
+  struct mad_pcm *pcm;
+  mad_fixed_t const *left_ch;
+  mad_fixed_t const *right_ch;
+  int16_t *output;
+  int nsamples;
+  int nchannels;
+  size_t bytes_read = 0;
+  size_t remaining = 0;
+  
+  if (!avc)
+    return 0;
+    
+  if (!mad)
+    return 0;
+      
+  remaining = mad->stream.bufend - mad->stream.next_frame;
+  memmove(mad->input_buffer, mad->stream.next_frame, remaining);
+  bytes_read = MIN(pkt->size, MAD_BUFSIZE - remaining);
+  memcpy(mad->input_buffer+remaining, pkt->data, bytes_read);
+  
+  if (bytes_read == 0){
+    *got_frame_ptr = 0;
+    return 0;
+  }
+  
+  mad_stream_buffer(&mad->stream, mad->input_buffer, remaining + bytes_read);
+  mad->stream.error = 0;
+  
+  if(!mad->got_header){
+    mad_header_decode(&mad->header, &mad->stream);
+    mad->got_header = 1;
+    avc->frame_size = 32 * (mad->header.layer == MAD_LAYER_I ? 12 :	\
+			    ((mad->header.layer == MAD_LAYER_III &&	\
+			      (mad->header.flags & MAD_FLAG_LSF_EXT)) ? 18 : 36));
+    avc->sample_fmt = AV_SAMPLE_FMT_S16;
+    if(mad->header.mode == MAD_MODE_SINGLE_CHANNEL){
+      avc->channel_layout = AV_CH_LAYOUT_MONO;
+      avc->channels = 1;
+    }else{
+      avc->channel_layout = AV_CH_LAYOUT_STEREO;
+      avc->channels = 2;
+    }
+  }
+
+  frame->channel_layout = avc->channel_layout;
+  frame->format = avc->sample_fmt;
+  frame->channels = avc->channels;
+  frame->nb_samples = avc->frame_size; 
+    
+  if ((ff_get_buffer(avc, frame, 0)) < 0)
+    return 0;
+ 
+  if (mad_frame_decode(&mad->frame, &mad->stream) == -1) {
+    *got_frame_ptr = 0;
+    return mad->stream.bufend - mad->stream.next_frame;
+  }
+
+  mad_synth_frame (&mad->synth, &mad->frame);
+  
+  pcm = &mad->synth.pcm;
+  output = (int16_t *)frame->data[0];
+  nsamples = pcm->length;
+  nchannels = pcm->channels;
+  left_ch = pcm->samples[0];
+  right_ch = pcm->samples[1];
+  while (nsamples--) {
+    *output++ = mad_scale(*(left_ch++));
+    if (nchannels == 2) {
+      *output++ = mad_scale(*(right_ch++));
+    }
+    //Players should recognise mono and play through both channels
+    //Writing the same thing to both left and right channels here causes
+    //memory issues as it creates double the number of samples allocated.
+  }
+  
+  *got_frame_ptr = 1;
+ 
+  return mad->stream.bufend - mad->stream.next_frame;
+}
+
+AVCodec ff_libmad_decoder = {
+    .name           = "libmad",
+    .long_name      = NULL_IF_CONFIG_SMALL("libmad MP3 decoder"),
+    .wrapper_name   = "libmad",
+    .type           = AVMEDIA_TYPE_AUDIO,
+    .id             = AV_CODEC_ID_MP3,
+    .sample_fmts    = (const enum AVSampleFormat[]) { AV_SAMPLE_FMT_S16, AV_SAMPLE_FMT_NONE },
+    .capabilities   = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_CHANNEL_CONF,
+    .priv_data_size = sizeof(libmad_context),
+    .init           = libmad_decode_init,
+    .close          = libmad_decode_close,
+    .decode         = libmad_decode_frame
+};
+

[-- Attachment #3: Type: text/plain, Size: 251 bytes --]

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".