[FFmpeg-devel] [PATCH 12/18] avformat/hls: parse ID3 timestamps for elementary audio streams

[Anssi Hannula]

03.01.2014 06:33, Michael Niedermayer kirjoitti:
> On Fri, Jan 03, 2014 at 04:21:03AM +0200, Anssi Hannula wrote:
>> 31.12.2013 09:15, Michael Niedermayer kirjoitti:
>>> On Tue, Dec 31, 2013 at 08:20:19AM +0200, Anssi Hannula wrote:
>>>> I seem to now be hitting "first_dts 895690 not matching first dts 920758
>>>> (pts 920758, duration 2089) in the queue" for every packet after the
>>>> first one (only the first one has actual dts), though...
>>>>
>>>> I have to quit debugging now for a while, but if you know off-hand what
>>>> I'm doing wrong (if anything - just noticed it is only AV_LOG_DEBUG) it
>>>> would save me some time :)
>>>
>>> if the timestamps you get are correct then you can probably ignore it
>>> this message can get printed when nothing is really wrong ...
>>>
>>>>
>>>> I have AVSTREAM_PARSE_FULL and I set pkt->dts for first pkt, and later
>>>> ones have AV_NOPTS_VALUE (the interspersed timestamps aren't yet handled).
>>
>>
>> Since there are several hundred non-timestamped packets between the
>> timestamped packets, the fact that compute_pkt_fields() rounds the
>> computed frame duration down (utils.c line ~1096) will cause the packet
>> timestamps to fall behind of the actual time (approx. ~5ms for 48kHz @
>> 1024 samples/pkt with 10sec segments,
> 
> this sounds a bit odd, mpeg has strict rules on how often timestamps
> must be stored
> without checking the spec i think the max duration between timestamps
> was below 1 sec
> so several hundread packets of 1024 samples sound like too much

Well this is just raw audio elementary stream (with no timestamps) with
HLS-specific ID3 timestamp in the beginning of every HLS segment (spec
says a "typical" segment duration is 10sec, but there is no limit).

I don't think the MPEG spec has much say in this, or does it?

I must admit I'm not 100% sure what the "MPEG-2 audio elementary stream
(ISO 13818)" format HLS spec says means exactly, but apparently it is
just raw audio, right? Just checking I'm not missing someting useful here...

>> since the pkt duration can't be
>> expressed exactly in MPEG timebase), so there will be a corresponding
>> jump in timestamps the next time a timestamped packet is output.
>>
>> (this didn't happen with my original patch since I kept the offset
>> between last timestamp and current packet around in the audio timebase,
>> not MPEG timebase, converting the full offset each time so that rounding
>> error did not accumulate)
>>
>> What should we do? Probably these are the reasonable options, unless you
>> think of something else:
> 
>> a) Not care.
> 
> thats a possible temporary solution
> 
> 
>> b) Add some artificial left-shift to all the timestamps in
>>    hls_id3_audio demuxer so that the rounding error will be negligible.
> 
>> c) Make compute_pkt_fields() keep track of the rounding error and
>>    correct pkt duration and/or dts accordingly.
> 
> i probably could look into this tomorrow if the timestamps are really
> that rare

OK, that'd be great. For now I'll go with not caring then.

BTW, just noticed something strange... When I have:
- one stream, with AVSTREAM_PARSE_FULL, and
- avformat_find_stream_info() done (by HLS demuxer),
and then do:
1. Output first packet with DTS=X, containing multiple actual frames
=> the AVPackets output from lavf get proper DTS = X, X+1, X+2
2. Output second packet no DTS:
=> now the AVPackets jump back to DTS = X, X+1, X+2
3. Output third packet, no DTS:
=> DTS continues correctly, X+3, X+4, X+5

And this jumping back only happens exactly once in the beginning of the
stream, everything is fine the next time a timestamped packet is output.

Anyway, I'll just workaround it for now by avoiding the DTS-less demuxer
packets by putting the whole segment (10sec) in the same AVPacket for
lavf to split. Better debug this when I have sent the code here...

>> d) Go back to intercepting the ID3 timestamps in the HLS demuxer
>>    instead of a separate subdemuxer, i.e. like my original PATCH 12 did
>>    (just fixing the issue it had). Then the subdemuxer is just raw
>>    demuxer and we can correct timestamps in hls demuxer like the patch
>>    did.
>>
>> I think I'd like (c) most, but I don't really want to delve into the
>> lavf timestamping code right now myself...
>> Then maybe (b) or (d), not sure.


-- 
Anssi Hannula