[FFmpeg-devel] [PATCH] lavd/pulse_audio_enc: fix timestamp calculation

[Lukasz M]

On 27 October 2013 20:34, Michael Niedermayer <michaelni at gmx.at> wrote:

> On Sat, Oct 26, 2013 at 11:23:02PM +0200, Lukasz M wrote:
> > On 26 October 2013 16:24, Michael Niedermayer <michaelni at gmx.at> wrote:
> >
> > > On Sat, Oct 26, 2013 at 03:47:13PM +0200, Lukasz M wrote:
> > > > On 26 October 2013 13:50, Michael Niedermayer <michaelni at gmx.at>
> wrote:
> > > >
> > > > > On Sat, Oct 26, 2013 at 12:53:53AM +0200, Lukasz Marek wrote:
> > > > > > Current implementation didn't include duration of
> > > > > > last processed packet.
> > > > > > Device may return negative timestamps without
> > > > > > this correction.
> > > > > >
> > > > > > Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> > > > > > ---
> > > > > >  libavdevice/pulse_audio_enc.c |   11 ++++++++++-
> > > > > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/libavdevice/pulse_audio_enc.c
> > > > > b/libavdevice/pulse_audio_enc.c
> > > > > > index 86ef648..a27ae3c 100644
> > > > > > --- a/libavdevice/pulse_audio_enc.c
> > > > > > +++ b/libavdevice/pulse_audio_enc.c
> > > > > > @@ -35,6 +35,7 @@ typedef struct PulseData {
> > > > > >      const char *device;
> > > > > >      pa_simple *pa;
> > > > > >      unsigned int stream_index;
> > > > > > +    int64_t timestamp_diff;
> > > > > >  } PulseData;
> > > > > >
> > > > > >  static av_cold int pulse_write_header(AVFormatContext *h)
> > > > > > @@ -115,6 +116,14 @@ static int
> pulse_write_packet(AVFormatContext
> > > *h,
> > > > > AVPacket *pkt)
> > > > > >      if (s->stream_index != pkt->stream_index)
> > > > > >          return 0;
> > > > > >
> > > > > > +    if (!(s->timestamp_diff = pkt->duration)) {
> > > > > > +        AVStream *st = h->streams[s->stream_index];
> > > > > > +        AVCodecContext *codec_ctx = st->codec;
> > > > > > +        AVRational r = { 1, codec_ctx->sample_rate };
> > > > > > +        int64_t samples = pkt->size /
> > > > > (av_get_bytes_per_sample(codec_ctx->sample_fmt) *
> codec_ctx->channels);
> > > > > > +        s->timestamp_diff = av_rescale_q(samples, r,
> st->time_base);
> > > > > > +    }
> > > > > > +
> > > > > >      if (pa_simple_write(s->pa, pkt->data, pkt->size, &error) <
> 0) {
> > > > > >          av_log(s, AV_LOG_ERROR, "pa_simple_write failed: %s\n",
> > > > > pa_strerror(error));
> > > > > >          return AVERROR(EIO);
> > > > >
> > > > > > @@ -128,7 +137,7 @@ static void
> > > > > pulse_get_output_timestamp(AVFormatContext *h, int stream, int64_t
> *
> > > > > >      PulseData *s = h->priv_data;
> > > > > >      pa_usec_t latency = pa_simple_get_latency(s->pa, NULL);
> > > > > >      *wall = av_gettime();
> > > > > > -    *dts = h->streams[s->stream_index]->cur_dts - latency;
> > > > > > +    *dts = h->streams[s->stream_index]->cur_dts +
> s->timestamp_diff
> > > -
> > > > > latency;
> > > > >
> > > > > its independant of this patch (which should be ok) but
> > > > > access to cur_dts from (de)muxer or in/outputs is not a very good
> idea
> > > > > cur_dts is maintained by the core code and may or may not be what
> > > > > a (de)muxer expects
> > >
> > > note, the cur_dts issue is more a demuxer/input side issue, its use
> > > in muxers is probably not that much of a problem currently
> > >
> > >
> > > > >
> > > >
> > > > I attached a patch that removes that cur_dts issue.
> > > > I have just question if there is any other way to get current pts
> than
> > > > pkt->pts in write_packet method?
> > >
> > > for raw pcm dts == pts
> > > dts should be set, and both should be equal
> > >
> > > if they are not set, cur_dts also would not be set to anything better
> > > as its based on them
> >
> >
> > Ok, thanks for hints.
> >
> > Just one thing. Doxy AVPacket says:
> >
> >     /**
> >      * Duration of this packet in AVStream->time_base units, 0 if
> unknown.
> >      * Equals next_pts - this_pts in presentation order.
> >      */
> >     int   duration;
> >
> > But duration is given in CodecContext->time_base.
> >
> > avcodec_encode_audio2 do:
> >
> > if (!avpkt->duration)
> >     avpkt->duration = ff_samples_to_time_base(avctx, frame->nb_samples);
> >
> > which is executed for pcm audio codec. pcm doesn't touch duration in
> packet.
> > I checked random encoder and it also set duration basing on
> AVCodecContext
> > time_base.
>
> encoders dont have access to AVStream
> ffmpeg.c should rescale the durations from AVCodecContext to AVStream
> timebases


Ok. Indeed ffmpeg tool provides correct duration. I used packets from
avcodec_encode_audio2 directly.
I attached a final patch just for a record. I have sent merge request for
this one along other patches.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavd-pulse_audio_enc-fix-timestamp-calculation.patch
Type: text/x-patch
Size: 2130 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131027/8f5dc39e/attachment.bin>