[FFmpeg-devel] [PATCH] ALSA for libavdevice

[Benoit Fouet]

Hi,

Nicolas George wrote :
> Hi. Thanks for your review.
>
>
> Le nonidi 19 frimaire, an CCXVII, Michael Niedermayer a ?crit :
>   
>
>>> +    av_log(s1, AV_LOG_WARNING, "XRUN!!!\n");
>>>       
>> This message can be improved i think
>>     
>
> "ALSA buffer underrun.\n"
>
>   

well, this could also be overrun... Michael, is it ok with a "ALSA
buffer xrun\n" ?

> diff --git a/libavdevice/alsa-audio-common.c b/libavdevice/alsa-audio-common.c
> new file mode 100644
> index 0000000..f80275e
> --- /dev/null
> +++ b/libavdevice/alsa-audio-common.c
> +int ff_alsa_open(AVFormatContext *ctx, int mode, unsigned int *sample_rate,
> +    int channels, int *codec_id)
> +{
> +    AlsaData *s = ctx->priv_data;
> +    const char *audio_device;
> +    int res, flags = 0;
> +    snd_pcm_format_t format;
> +    snd_pcm_t *h;
> +    snd_pcm_hw_params_t *hw_params;
> +    snd_pcm_uframes_t buffer_size, period_size;
> +
> +    if (ctx->filename[0] == 0) {
> +        audio_device = "default";
> +    } else {
> +        audio_device = ctx->filename;
> +    }
> +
> +    if (*codec_id == CODEC_ID_NONE)
> +        *codec_id = DEFAULT_CODEC_ID;
> +    format = codec_id_to_pcm_format(*codec_id);
> +    if (format == SND_PCM_FORMAT_UNKNOWN) {
> +        av_log(ctx, AV_LOG_ERROR, "sample format %x is not supported\n", *codec_id);
>   

%d

> +int ff_alsa_xrun_recover(AVFormatContext *s1, int err)
> +{
> +    AlsaData *s = s1->priv_data;
> +    snd_pcm_t *handle = s->h;
> +
> +    av_log(s1, AV_LOG_WARNING, "ALSA buffer underrun.\n");
> +    if (err == -EPIPE) {
> +        err = snd_pcm_prepare(handle);
> +        if (err < 0) {
> +            av_log(s1, AV_LOG_ERROR, "cannot recover from underrun (snd_pcm_prepare failed: %s)\n", snd_strerror(err));
> +
> +            return 0;
>   

shouldn't that be a negative value ?

> +        }
> +    } else if (err == -ESTRPIPE) {
> +        av_log(NULL, AV_LOG_ERROR, "-ESTPIPE... Unsupported!\n");
> +
>   

typo

> diff --git a/libavdevice/alsa-audio-dec.c b/libavdevice/alsa-audio-dec.c
> new file mode 100644
> index 0000000..4123508
> --- /dev/null
> +++ b/libavdevice/alsa-audio-dec.c
> +static int audio_read_header(AVFormatContext *s1, AVFormatParameters *ap)
> +{
> +    AlsaData *s = s1->priv_data;
> +    AVStream *st;
> +    int ret;
> +    unsigned int sample_rate;
> +    int codec_id;
> +    snd_pcm_sw_params_t *sw_params;
> +
> +    if (ap->sample_rate <= 0 || ap->channels <= 0) {
> +        av_log(s1, AV_LOG_ERROR, "Bad sample rate %d or channels number %d\n",
> +                   ap->sample_rate, ap->channels);
> +
> +        return AVERROR(EIO);
> +    }
> +
>   

maybe we could split it in the first place in two different errors ?

[...]

> +#include <sys/time.h>
> +
>   

I'd prefer to see it on the top of the file (if you still need it)

> +static int audio_read_packet(AVFormatContext *s1, AVPacket *pkt)
> +{
> +    AlsaData *s = s1->priv_data;
> +    AVStream *st = s1->streams[0];
> +    int res;
> +    snd_htimestamp_t timestamp;
> +    snd_pcm_uframes_t ts_delay;
> +
> +
> +    if (av_new_packet(pkt, s->period_size) < 0) {
> +        return AVERROR(EIO);
> +    }
> +
> +    while ((res = snd_pcm_readi(s->h, pkt->data, pkt->size / s->frame_size)) < 0) {
> +        if (res == -EAGAIN) {
> +            pkt->size = 0;
> +            av_free_packet(pkt);
> +
> +            return AVERROR(EAGAIN);
> +        }
> +        if (ff_alsa_xrun_recover(s1, res) < 0) {
> +                av_log(s1, AV_LOG_ERROR, "Alsa read error: %s\n",
> +                                   snd_strerror(res));
> +                av_free_packet(pkt);
> +
> +                return AVERROR(EIO);
> +        }
> +    }
> +
> +    snd_pcm_htimestamp(s->h, &ts_delay, &timestamp);
> +    ts_delay += res;
> +    pkt->pts = (int64_t)timestamp.tv_sec * 1000000 + timestamp.tv_nsec / 1000;
> +    pkt->pts -= (int64_t)ts_delay * 1000000 / st->codec->sample_rate;
> +
> +    pkt->size = res * s->frame_size;
> +
> +    return 0;
> +}
> +
> +AVInputFormat alsa_demuxer = {
> +    "alsa",
> +    "Alsa audio input",
>   

this lacks the NULL_IF_CONFIG_SMALL

> +    sizeof(AlsaData),
> +    NULL,
> +    audio_read_header,
> +    audio_read_packet,
> +    ff_alsa_close,
> +    .flags = AVFMT_NOFILE,
> +};
> diff --git a/libavdevice/alsa-audio-enc.c b/libavdevice/alsa-audio-enc.c
> new file mode 100644
> index 0000000..ce9eb5d
> --- /dev/null
> +++ b/libavdevice/alsa-audio-enc.c
> +AVOutputFormat alsa_muxer = {
> +    "alsa",
> +    "Alsa audio output",
>   

ditto

BTW, thanks for doing this !

Ben