[FFmpeg-devel] [PATCH] avdevice/pulse_audio_dec: reimplement using the non simple API
Michael Niedermayer
michaelni at gmx.at
Wed Jul 9 01:23:47 CEST 2014
On Tue, Jul 08, 2014 at 12:38:03PM +0200, Lukasz Marek wrote:
> On 8 July 2014 05:50, Michael Niedermayer <michaelni at gmx.at> wrote:
>
> > This fixes timestamps
> >
> > Based-on: code from pulseaudio
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> > libavdevice/pulse_audio_dec.c | 284
> > +++++++++++++++++++++++++++++++++--------
> > 1 file changed, 232 insertions(+), 52 deletions(-)
> >
> > diff --git a/libavdevice/pulse_audio_dec.c b/libavdevice/pulse_audio_dec.c
> > index 414d73b..756ef4c 100644
> > --- a/libavdevice/pulse_audio_dec.c
> > +++ b/libavdevice/pulse_audio_dec.c
> > @@ -1,6 +1,8 @@
> > /*
> > * Pulseaudio input
> > * Copyright (c) 2011 Luca Barbato <lu_zero at gentoo.org>
> > + * Copyright 2004-2006 Lennart Poettering
> > + * Copyright (c) 2014 Michael Niedermayer <michaelni at gmx.at>
> > *
> > * This file is part of FFmpeg.
> > *
> > @@ -19,19 +21,15 @@
> > * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> > 02110-1301 USA
> > */
> >
> > -/**
> > - * @file
> > - * PulseAudio input using the simple API.
> > - * @author Luca Barbato <lu_zero at gentoo.org>
> > - */
> > -
> > #include <pulse/simple.h>
> >
>
> You can remove this header.
removed
>
>
> > #include <pulse/rtclock.h>
> > #include <pulse/error.h>
> > #include "libavformat/avformat.h"
> > #include "libavformat/internal.h"
> > #include "libavutil/opt.h"
> > +#include "libavutil/time.h"
> > #include "pulse_audio_common.h"
> > +#include "timefilter.h"
> >
> > #define DEFAULT_CODEC_ID AV_NE(AV_CODEC_ID_PCM_S16BE,
> > AV_CODEC_ID_PCM_S16LE)
> >
> > @@ -44,17 +42,103 @@ typedef struct PulseData {
> > int channels;
> > int frame_size;
> > int fragment_size;
> > - pa_simple *s;
> > - int64_t pts;
> > - int64_t frame_duration;
> > +
> > + pa_threaded_mainloop *mainloop;
> > + pa_context *context;
> > + pa_stream *stream;
> > +
> > + TimeFilter *timefilter;
> > + int last_period;
> > } PulseData;
> >
> > +
> > +#define CHECK_SUCCESS_GOTO(p, rerror, expression, label) \
> >
>
> p is not used.
removed
>
>
> > + do { \
> > + if (!(expression)) { \
> > + rerror = -1; \
> > + goto label; \
> > + } \
> > + } while(0);
> > +
> > +#define CHECK_DEAD_GOTO(p, rerror, label) \
> > + do { \
> > + if (!(p)->context ||
> > !PA_CONTEXT_IS_GOOD(pa_context_get_state((p)->context)) || \
> > + !(p)->stream ||
> > !PA_STREAM_IS_GOOD(pa_stream_get_state((p)->stream))) { \
> > + if (((p)->context && pa_context_get_state((p)->context) ==
> > PA_CONTEXT_FAILED) || \
> > + ((p)->stream && pa_stream_get_state((p)->stream) ==
> > PA_STREAM_FAILED)) { \
> > + rerror = -1; \
> > + } else \
> > + rerror = -1; \
> >
>
> Is this "if else" really needed?
no, removed
> If -1 is error code than maybe AVERROR_EXTERNAL?, here and above.
changed
>
>
> > + goto label; \
> > + } \
> > + } while(0);
> > +
> >
> [...]
>
> > +static av_cold int pulse_close(AVFormatContext *s)
> > +{
> > + PulseData *pd = s->priv_data;
> > +
> > + if (pd->mainloop)
> > + pa_threaded_mainloop_stop(pd->mainloop);
> > +
> > + if (pd->stream)
> > + pa_stream_unref(pd->stream);
> > +
> > + if (pd->context) {
> > + pa_context_disconnect(pd->context);
> > + pa_context_unref(pd->context);
> > + }
> > +
> > + if (pd->mainloop)
> > + pa_threaded_mainloop_free(pd->mainloop);
> >
>
> You can set to NULL pd->mainloop, pd->context and pd->stream to make this
> function idiot proof.
wanted to do already, seems i forgotton
changed now
>
>
> > +
> > + ff_timefilter_destroy(pd->timefilter);
> > + pd->timefilter = NULL;
> > +
> > + return 0;
> > +}
> > +
> > static av_cold int pulse_read_header(AVFormatContext *s)
> > {
> > PulseData *pd = s->priv_data;
> > AVStream *st;
> > char *device = NULL;
> > - int ret, sample_bytes;
> > + int ret;
> > enum AVCodecID codec_id =
> > s->audio_codec_id == AV_CODEC_ID_NONE ? DEFAULT_CODEC_ID :
> > s->audio_codec_id;
> > const pa_sample_spec ss = { ff_codec_id_to_pulse_format(codec_id),
> > @@ -75,77 +159,173 @@ static av_cold int pulse_read_header(AVFormatContext
> > *s)
> > if (strcmp(s->filename, "default"))
> > device = s->filename;
> >
> > - pd->s = pa_simple_new(pd->server, pd->name,
> > - PA_STREAM_RECORD,
> > - device, pd->stream_name, &ss,
> > - NULL, &attr, &ret);
> > + if (!(pd->mainloop = pa_threaded_mainloop_new())) {
> > + pulse_close(s);
> > + return -1;
> >
>
> Maybe AVERROR_EXTERNAL?
>
>
> > + }
> > +
> > + if (!(pd->context =
> > pa_context_new(pa_threaded_mainloop_get_api(pd->mainloop), pd->name))) {
> > + pulse_close(s);
> > + return -1;
> >
>
> Maybe AVERROR_EXTERNAL?
>
>
> > + }
> > +
> > + pa_context_set_state_callback(pd->context, context_state_cb, pd);
> > +
> > + if (pa_context_connect(pd->context, pd->server, 0, NULL) < 0) {
> > + pulse_close(s);
> > + return AVERROR(pa_context_errno(pd->context));
> > + }
> > +
> > + pa_threaded_mainloop_lock(pd->mainloop);
> >
>
> [...]
>
>
> > - pd->pts += pd->frame_duration;
> > + if (pa_stream_get_latency(pd->stream, &latency, &negative) >= 0) {
> > + enum AVCodecID codec_id =
> > + s->audio_codec_id == AV_CODEC_ID_NONE ? DEFAULT_CODEC_ID :
> > s->audio_codec_id;
> > + int frame_size = ((av_get_bits_per_sample(codec_id) >> 3) *
> > pd->channels);
> > + int frame_duration = read_length / frame_size;
> >
> > - return 0;
> > -}
> >
> > -static av_cold int pulse_close(AVFormatContext *s)
> > -{
> > - PulseData *pd = s->priv_data;
> > - pa_simple_free(pd->s);
> > - pd->s = NULL;
> > + dts -= latency;
> >
>
> you should take negative variable into account too. As far as I remember it
> may be set for record streams when monitor device is recorded.
fixed
patch applied
thanks
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
What does censorship reveal? It reveals fear. -- Julian Assange
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140709/9c298f64/attachment.asc>
More information about the ffmpeg-devel
mailing list