Opened 21 months ago

Last modified 14 months ago

#8522 open defect

DASH start segment number calculation is wrong for fragment_duration mode

Reported by: onitake Owned by: Steven Liu
Priority: normal Component: avformat
Version: git-master Keywords: dash
Cc: lq@onvideo.cn, spoeschel@irt.de Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:

When playing an MPEG-DASH live stream in fragment_duration, the calculation of the first segment is incorrect, which can result in a significant offset.

In a test (as outlined below) with timescale=48000 and duration=288000, I can observe an offset of 3 days in the future. This will clearly not work, as it's very unlikely that live stream segments are produced 3 days in advance.

How to reproduce:

This is a private test stream and cannot be published here, unfortunately.
The anonymized DASH manifest is attached.

% ffmpeg -v verbose -re -i https://<REDACTED>/dash.mpd -c:a copy -c:v copy -f mpegts -y live.ts

ffmpeg version 4.2.2-1 Copyright (c) 2000-2019 the FFmpeg developers
  built with gcc 9 (Debian 9.2.1-24)

Observations:

With this manifest, there is an offset of several days between the segments that are requested by ffmpeg and what is available on the streaming CDN.

When I ran a test on Monday (when the stream was set up), I ended up with a request for segment 263604602, while the DASH-IF reference player (https://reference.dashif.org/dash.js/v2.9.1/samples/dash-if-reference-player/index.html) requested segment 263557058, which is correct and works.

I ran the same test today and got 263605211 on ffmpeg and 263588024 on the DASH-IF player.

Source of the issue:

The issue lies in how the first segment number is calculated in dashdec.c:

    if (c->is_live) {
        // ...
        } else if (pls->fragment_duration){
            // ...
            } else if (c->publish_time > 0 && !c->availability_start_time) {
                if (c->min_buffer_time) {
                    num = pls->first_seq_no + (((c->publish_time + pls->fragment_duration) - c->suggested_presentation_delay) * pls->fragment_timescale) / pls->fragment_duration - c->min_buffer_time;
                } else {
                    num = pls->first_seq_no + (((c->publish_time - c->time_shift_buffer_depth + pls->fragment_duration) - c->suggested_presentation_delay) * pls->fragment_timescale) / pls->fragment_duration;
                }

Here, the fragment duration in timescale units is added to the first segment number, which is in seconds, not timescale units.
The resulting time offset is then scaled to the segment number by multiplying with the timescale and dividing by the fragment duration in timescale units.
The end result is that fragment_duration * fragment_timescale / fragment_duration = fragment_timescale is added to the segment number, which doesn't make much sense.

In my case (where first_seq_no=0, timescale=48000 and duration=288000), this results in a shift of 48000 segment numbers, which corresponds to 288000 seconds here, and that is 3.333 days, as I originally observed during my first tests.

Furthermore, I'm almost 100% certain that the publish time is not relevant to the segment number in any way for live streams. Only the current time and the availability_start_time should be used.
If I remove the fragment_timescale shift, I still end up with incorrect segment numbers, because they are now relative to the publish time.
At least, it looks like the DASH-IF reference player seems to ignore the publish time and only loads segments relative to the availability start time.

The specification describes the publishTime attribute as follows:

specifies the wall-clock time when the MPD was
generated and published at the origin server. MPDs with
a later value of @publishTime shall be an update as
defined in 5.4 to MPDs with earlier @publishTime.

As a test, I removed the whole special casing for 'c->publish_time > 0 && !c->availability_start_time' and now my stream works correctly.

Addendum:

I will try to obtain permission to publish this stream publicly, but I cannot do that at the moment.

Attachments (3)

dash.mpd (1.9 KB ) - added by onitake 21 months ago.
manifest.mpd (2.1 KB ) - added by Stefan Pöschel 14 months ago.
0001-Remove-DASH-decoder-special-case-for-publish_time-0-.patch (1.9 KB ) - added by onitake 14 months ago.

Download all attachments as: .zip

Change History (18)

by onitake, 21 months ago

Attachment: dash.mpd added

comment:1 by onitake, 21 months ago

Clarification:

When I ran a test on Monday (when the stream was set up), I ended up with a request for segment 263604602, while the DASH-IF reference player (​https://reference.dashif.org/dash.js/v2.9.1/samples/dash-if-reference-player/index.html) requested segment 263557058, which is correct and works.

I ran the same test today and got 263605211 on ffmpeg and 263588024 on the DASH-IF player.

The segment that is requested by the DASH-IF player is correct and works.
ffmpeg requests segments that are not available on the CDN.

comment:2 by Carl Eugen Hoyos, 21 months ago

Keywords: mpd removed
Priority: importantnormal
Version: 4.2unspecified

Please test current FFmpeg git head and provide the command line you tested together with the complete, uncut console output to make this a valid ticket.

comment:3 by onitake, 21 months ago

Version: unspecifiedgit-master

comment:4 by onitake, 21 months ago

I'm sorry, I actually tested and traced this on the latest git HEAD.
The version I posted above was from the system-installed ffmpeg instance, where the issue was first discovered. The relevant code section is unchanged between those two versions, as far as I can see.

Console output will be provided as soon as I'm back on my test workstation.

in reply to:  4 comment:5 by Steven Liu, 21 months ago

Cc: lq@onvideo.cn added
Owner: set to Steven Liu
Status: newopen

Replying to onitake:

I'm sorry, I actually tested and traced this on the latest git HEAD.
The version I posted above was from the system-installed ffmpeg instance, where the issue was first discovered. The relevant code section is unchanged between those two versions, as far as I can see.

Console output will be provided as soon as I'm back on my test workstation.

Waiting for your test stream online, i cannot get the point, i need reproduce it by myself.

comment:6 by onitake, 21 months ago

Can I mail you the stream URL directly, Steven?
The test stream is generated from broadcast TV, and licensing is a bit of an issue. I'd rather not post the URL here in public.

in reply to:  6 comment:7 by Steven Liu, 21 months ago

Replying to onitake:

Can I mail you the stream URL directly, Steven?
The test stream is generated from broadcast TV, and licensing is a bit of an issue. I'd rather not post the URL here in public.

Sure. Thanks onitake.

comment:8 by onitake, 20 months ago

@stevenliu Were you able to test with the URL I sent you?
I'm not sure how long I can keep this stream online.

If you have a possibility to set up a DASH stream yourself, the important points to trigger the issue are:

availabilityStartTime="1970-01-01T00:00:00Z"
publishTime="2020-02-10T14:02:08Z" (or any other value != "1970-01-01T00:00:00Z")

If you want to have the exact same setup, you also need to set:

timescale="48000"
duration="288000"

comment:9 by onitake, 20 months ago

I cannot keep the test stream running any longer, unfortunately.

It should be possible to generate a stream with the mentioned parameters with MP4Box or FFmpeg, but I currently don't know which options would need to be passed to achieve this.

comment:10 by Stefan Pöschel, 14 months ago

Cc: spoeschel@irt.de added

I can confirm this issue and also, that @publishTime is actually irrelevant for the calculation of the current segment number.

I tested with Git HEAD and the following stream (geoblocked to Germany; therefore attaching the MPD):

http://rbbdashlive-i.akamaihd.net/dash/live/481522/berlin/manifest.mpd

by Stefan Pöschel, 14 months ago

Attachment: manifest.mpd added

comment:11 by onitake, 14 months ago

I attached a patch that removes the incorrect special case completely.
It's unclear why this special case was there in the first place, it doesn't seem to correspond to anything in the spec.

comment:12 by Stefan Pöschel, 14 months ago

On this occasion it could make sense to refactor this function in general, as the remaining calculations have many parts in common.

in reply to:  11 comment:13 by Balling, 14 months ago

Replying to onitake:

I attached a patch that removes the incorrect special case completely.
It's unclear why this special case was there in the first place, it doesn't seem to correspond to anything in the spec.

There is this thing called git blame, here is a commit that implemented some of it. 8eac027cd14eb507d7d1bddf2606a01e1d118d38

comment:14 by Stefan Pöschel, 14 months ago

The usage of the publish time actually was already part of the initial version.

in reply to:  14 comment:15 by Balling, 14 months ago

Replying to spoeschel:

The usage of the publish time actually was already part of the initial version.

What about c->min_buffer_time?? If you will delete it, it will not be used at all except for initial parsing... That looks wrong to me...

Note: See TracTickets for help on using tickets.