[FFmpeg-devel] [PATCH 1/2] lavd/alsa: fix timestamp calculation

[Lukasz M]

On 26 October 2013 15:49, Lukasz M <lukasz.m.luki at gmail.com> wrote:

> On 26 October 2013 11:46, Nicolas George <george at nsup.org> wrote:
>
>> Le quintidi 5 brumaire, an CCXXII, Lukasz M a écrit :
>> > From 3d7590ed1d012367f0e1408efa4b629d6343220c Mon Sep 17 00:00:00 2001
>> > From: Lukasz Marek <lukasz.m.luki at gmail.com>
>> > Date: Sat, 26 Oct 2013 01:19:31 +0200
>> > Subject: [PATCH 1/2] lavd/alsa: fix timestamp calculation
>> >
>> > 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/alsa-audio-enc.c |    9 ++++++++-
>> >  libavdevice/alsa-audio.h     |    1 +
>> >  2 files changed, 9 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/libavdevice/alsa-audio-enc.c b/libavdevice/alsa-audio-enc.c
>> > index 0f4e4a2..2239046 100644
>> > --- a/libavdevice/alsa-audio-enc.c
>> > +++ b/libavdevice/alsa-audio-enc.c
>> > @@ -79,6 +79,13 @@ static int audio_write_packet(AVFormatContext *s1,
>> AVPacket *pkt)
>> >      int size     = pkt->size;
>> >      uint8_t *buf = pkt->data;
>> >
>> > +    if (!(s->timestamp_diff = pkt->duration)) {
>> > +        AVStream *st = s1->streams[0];
>> > +        AVCodecContext *codec_ctx = st->codec;
>> > +        /* XXX: no need to recalculate: 1/sample_rate ==
>> avpriv_set_pts_info() */
>>
>> > +        s->timestamp_diff = pkt->size /
>> (av_get_bytes_per_sample(codec_ctx->sample_fmt) * codec_ctx->channels);
>>
>> The number of bytes per sample is already in s->frame_size. In fact:
>>
>> > +    }
>> > +
>> >      size /= s->frame_size;
>>
>> ... the duration is precisely "size" now.
>>
>> Therefore, you could probably just write:
>>
>>     s->timestamp_diff = pkt->duration ? pkt->duration : size;
>>
>> (and curse C not to be able to write "pkt->duration || size").
>>
>> >      if (s->reorder_func) {
>> >          if (size > s->reorder_buf_size)
>> > @@ -112,7 +119,7 @@ audio_get_output_timestamp(AVFormatContext *s1, int
>> stream,
>> >      snd_pcm_sframes_t delay = 0;
>> >      *wall = av_gettime();
>> >      snd_pcm_delay(s->h, &delay);
>> > -    *dts = s1->streams[0]->cur_dts - delay;
>> > +    *dts = s1->streams[0]->cur_dts + s->timestamp_diff - delay;
>> >  }
>> >
>> >  AVOutputFormat ff_alsa_muxer = {
>> > diff --git a/libavdevice/alsa-audio.h b/libavdevice/alsa-audio.h
>> > index 44b7c72..005cf83 100644
>> > --- a/libavdevice/alsa-audio.h
>> > +++ b/libavdevice/alsa-audio.h
>> > @@ -57,6 +57,7 @@ typedef struct AlsaData {
>> >      void (*reorder_func)(const void *, void *, int);
>> >      void *reorder_buf;
>> >      int reorder_buf_size; ///< in frames
>> > +    int64_t timestamp_diff; ///< duration of last packet, needed to
>> calculate timestamp
>> >  } AlsaData;
>> >
>> >  /**
>>
>> Apart from that simplification, it looks right, thanks.
>>
>
> I will fix it as you pointed, but for now Michael pointed at similar patch
> that st->cur_dts is not good idea to use.
> I will resubmit new patch when it is discussed basing on pulse version.
>

Fixed patch attached. Some additional changes added.
For reffrence you can also have a look on thread regarding the same change
in pulse audio.
http://ffmpeg.org/pipermail/ffmpeg-devel/2013-October/150006.html
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lavd-alsa-fix-timestamp-calculation.patch
Type: text/x-patch
Size: 1835 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20131027/e309780a/attachment.bin>