Closed Bug 1269325 Opened 8 years ago Closed 8 years ago

Incorrect buffered range with site

Categories

(Core :: Audio/Video: Playback, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla49
Tracking Status
firefox46 --- wontfix
firefox47 --- fixed
firefox48 --- fixed
firefox49 --- fixed

People

(Reporter: jya, Assigned: jya)

References

(Blocks 1 open bug, Regression, )

Details

Attachments

(1 file)

https://people.mozilla.org/~jyavenard/tests/mse_mp4/wowza-stall.html

Buffered range is returned as :

{{ 0,2.668977 }{ 2.999288,3.036 }}

it should be:
{{ 0,2.668977 }{ 2.999288,5.472045 }}

There's a problem with the dts of this file that cause the gap detection to be triggered, to be investigated.

I suspect something to do with the moof parser
Actually, our implementation is perfectly up to spec, it's Safari / Chrome that aren't.

We determine the dts exactly like what ffmpeg calculates:

jyambp:wowza-stall jyavenard$ ffprobe -show_entries packet=dts_time,duration_time t.mp4 
ffprobe version 2.8 Copyright (c) 2007-2015 the FFmpeg developers
  built with Apple LLVM version 7.0.2 (clang-700.1.81)
  configuration: --prefix=/opt/local --enable-swscale --enable-avfilter --enable-avresample --enable-libmp3lame --enable-libvorbis --enable-libopus --enable-libtheora --enable-libschroedinger --enable-libopenjpeg --enable-libmodplug --enable-libvpx --enable-libspeex --enable-libass --enable-libbluray --enable-lzma --enable-gnutls --enable-fontconfig --enable-libfreetype --enable-libfribidi --disable-indev=jack --disable-outdev=xv --mandir=/opt/local/share/man --enable-shared --enable-pthreads --cc=/usr/bin/clang --enable-vda --arch=x86_64 --enable-yasm --enable-gpl --enable-postproc --enable-libx264 --enable-libxvid
  libavutil      54. 31.100 / 54. 31.100
  libavcodec     56. 60.100 / 56. 60.100
  libavformat    56. 40.101 / 56. 40.101
  libavdevice    56.  4.100 / 56.  4.100
  libavfilter     5. 40.101 /  5. 40.101
  libavresample   2.  1.  0 /  2.  1.  0
  libswscale      3.  1.101 /  3.  1.101
  libswresample   1.  2.101 /  1.  2.101
  libpostproc    53.  3.100 / 53.  3.100
Input #0, mov,mp4,m4a,3gp,3g2,mj2, from 't.mp4':
  Metadata:
    major_brand     : isom
    minor_version   : 1
    compatible_brands: isomavc1
    creation_time   : 1970-01-01 00:00:01
  Duration: 00:00:05.47, start: 0.000000, bitrate: 137 kb/s
    Stream #0:0(und): Video: h264 (Main) (avc1 / 0x31637661), yuv420p, 1280x720 [SAR 1:1 DAR 16:9], 135 kb/s, 5.48 fps, 29.97 tbr, 22500 tbn, 10 tbc (default)
    Metadata:
      creation_time   : 1970-01-01 00:00:02
      handler_name    : VideoHandler
      encoder         : dailymotion/hls.js
[PACKET]
dts_time=0.000000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=0.200000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=0.233022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=0.601022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=0.801022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=0.834000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=1.201022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=1.402000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=1.435022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=1.802000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=2.002000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=2.035022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=2.403022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=2.603022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=2.636000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=2.668978
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=3.036000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=3.203022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=3.236000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=3.269022
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=3.804000
duration_time=N/A
[/PACKET]
[PACKET]
dts_time=4.004000
duration_time=0.033333
[/PACKET]
[PACKET]
dts_time=4.037022
duration_time=0.033333
[/PACKET]
[PACKET]
dts_time=4.070000
duration_time=0.033333
[/PACKET]
[PACKET]
dts_time=4.605022
duration_time=0.033333
[/PACKET]
[PACKET]
dts_time=4.805022
duration_time=0.033333
[/PACKET]
[PACKET]
dts_time=4.838000
duration_time=0.033333
[/PACKET]
[PACKET]
dts_time=5.205022
duration_time=0.033333
[/PACKET]
[PACKET]
dts_time=5.406000
duration_time=0.033333
[/PACKET]
[PACKET]
dts_time=5.439022
duration_time=0.033333
[/PACKET]

At end time 3.036, we have a dts of 2.668977, the duration of that sample is 0.036712. The next sample has a dts of 3.036000

the difference between those two dts time is 3.036000-2.668977 = 0.367023s

Per spec:
https://w3c.github.io/media-source/#sourcebuffer-coded-frame-processing
step 6:
"If last decode timestamp for track buffer is set and the difference between decode timestamp and last decode timestamp is greater than 2 times last frame duration:
 [snip]
 5. Set the need random access point flag on all track buffers to true."

that the need random access point is set mean that all frames will be ignored until one is found that is a keyframe as per step 10:
"10. If the need random access point flag on track buffer equals true, then run the following steps:

    1. If the coded frame is not a random access point, then drop the coded frame and jump to the top of the loop to start processing the next coded frame.
    2. Set the need random access point flag on track buffer to false.
"

the difference of 0.367023s is almost 10 times more than the previous duration....

Fix your timestamps, they aren't valid. See in ISOBMFF 14496-12, chapter 8.15.1 for the definition of time to sample boxes:
"The value of DT for a sample is always the sum of the deltas of the preceding samples. Note that the total of the decoding deltas is the duration of the media in this track."

here the sum of the deltas (e.g. the duration) isn't equal to the duration of the media in this track.
duration = dts_next - dts_previous
or: dts_next = dts_previous + duration
whichever way you want to see it.

The dts are bad in the original ts file too, but broken ts in mpeg-ts are not unheard of.

hls.js need to ensure it doesn't produce file with invalid dts.

I don't think we should loosen our MSE implentation simply to get around badly muxed file
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(g.du.pontavice)
Resolution: --- → INVALID
the original TS files can be found there:
https://people.mozilla.org/~jyavenard/mediatest/hls/s2/

Two ways to fix this:

1- Fix the TS encoder used to produce the original files
2- Have hls.js rewrite the dts so they are valid.
This bug is identical to bug 1268868 ; following bug 1268868, hls.js will be able to detect the gaps and will skip over them normally. But would be better to not have gaps in the first place
See Also: → 1268868
Upon reflection, a quick fix would be for samples that do not have a duration, to always use the delta of the 2 dts.

example:
video/avc frame(pts:2999288 end:3036000, dts:2668977, duration:36712, kf:1)
video/avc frame(pts:3036000 end:3203022, dts:3036000, duration:167022, kf:0)
video/avc frame(pts:3203022 end:3236000, dts:3203022, duration:32978, kf:0)

The first frame should have a duration of 167022 (3.036000 - 2.668977 = 0.367023)

Duration aren't that important anyway as only the pts is used during composition.
I'll reopen, because the issue is made worse with how we handle the cts/dts in the moof parser.

In order to ensure that all samples in a moof are contiguous, the cts are adjusted so that cts end is always cts / start

but the dts aren't adjusted and as such do not increment by the duration.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
CTS are adjusted so that all frames within a moof are contiguous and gapless. This means that the duration of each sample are updated accordingly. In MP4, the definition of a sample's duration is the delta between two decoding timestamp. As such, when changing the duration, the decode timestamp should be updated accordingly.

Review commit: https://reviewboard.mozilla.org/r/50131/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/50131/
Attachment #8748060 - Flags: review?(ajones)
Assignee: nobody → jyavenard
this is the primary cause of several stalls reported by hls.js user and other MSE sites, including bug 1268868
Flags: needinfo?(g.du.pontavice)
Comment on attachment 8748060 [details]
MozReview Request: Bug 1269325: [mp4] Recalculate dts after adjusting cts. r?kentuckyfriedtakahe

https://reviewboard.mozilla.org/r/50131/#review47079

I'm good with this fix but as much as I hate to say it, this change probably deserves a comment to explain why we're doing it.
Attachment #8748060 - Flags: review?(ajones) → review+
Comment on attachment 8748060 [details]
MozReview Request: Bug 1269325: [mp4] Recalculate dts after adjusting cts. r?kentuckyfriedtakahe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50131/diff/1-2/
Comment on attachment 8748060 [details]
MozReview Request: Bug 1269325: [mp4] Recalculate dts after adjusting cts. r?kentuckyfriedtakahe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/50131/diff/2-3/
@jyavenard@mozilla.com hey I am really thankful for the support given by you,I tested on latest build given by you , Here I could reproduce the issue too :( . 

Here are my findings,this is the link that has ts chunks and chunklist.m3u8 https://drive.google.com/open?id=0BzmeWR8wPIT0VzhpaFVtTFRMZ1E where you can reproduce the issue.

I combined all ts chunks using this command "ffmpeg -i  'concat:media_b400000_318.ts|media_b400000_319.ts|media_b400000_320.ts' -vcodec copy output123.ts "

Then converted  output123.ts to .mp4 using "ffmpeg-3.0-64bit-static/ffmpeg -i output123.ts -vcodec copy output.mp4"

Ran this command to compare the dts_values ./ffprobe -show_entries packet=dts_time,duration_time ~/charles_ts/ff_mse_chunks/output.mp4

./ffprobe -show_entries packet=dts_time,duration_time ~/charles_ts/ff_mse_chunks/output.mp4
ffprobe version 3.0-static http://johnvansickle.com/ffmpeg/  Copyright (c) 2007-2016 the FFmpeg developers
  built with gcc 5.3.1 (Debian 5.3.1-8) 20160205
  configuration: --enable-gpl --enable-version3 --disable-shared --disable-debug --enable-runtime-cpudetect --enable-libmp3lame --enable-libx264 --enable-libx265 --enable-libwebp --enable-libspeex --enable-libvorbis --enable-libvpx --enable-libfreetype --enable-fontconfig --enable-libxvid --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libtheora --enable-libvo-amrwbenc --enable-gray --enable-libopenjpeg --enable-libopus --enable-libass --enable-gnutls --enable-libvidstab --enable-libsoxr --enable-frei0r --enable-libfribidi --disable-indev=sndio --disable-outdev=sndio --enable-librtmp --enable-libmfx --cc=gcc
  libavutil      55. 17.103 / 55. 17.103
  libavcodec     57. 24.102 / 57. 24.102
  libavformat    57. 25.100 / 57. 25.100
  libavdevice    57.  0.101 / 57.  0.101
  libavfilter     6. 31.100 /  6. 31.100
  libswscale      4.  0.100 /  4.  0.100
  libswresample   2.  0.101 /  2.  0.101
  libpostproc    54.  0.100 / 54.  0.100
Input #0, mov,mp4,m4a,3gp,3g2,mj2, from '/home/denimuser/charles_ts/ff_mse_chunks/output.mp4':
  Metadata:
    major_brand     : isom
    minor_version   : 512
    compatible_brands: isomiso2avc1mp41
    encoder         : Lavf57.25.100
  Duration: 00:00:09.00, start: 0.000000, bitrate: 437 kb/s
    Stream #0:0(und): Video: h264 (Main) (avc1 / 0x31637661), yuv420p(tv), 1280x720 [SAR 1:1 DAR 16:9], 436 kb/s, 5 fps, 5 tbr, 90k tbn, 10 tbc (default)
    Metadata:
      handler_name    : VideoHandler
[PACKET]
dts_time=0.000000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=0.200000
duration_time=0.219000
[/PACKET]
[PACKET]
dts_time=0.419000
duration_time=0.178000
[/PACKET]
[PACKET]
dts_time=0.597000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=0.797000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=0.997000
duration_time=0.202000
[/PACKET]
[PACKET]
dts_time=1.199000
duration_time=0.193000
[/PACKET]
[PACKET]
dts_time=1.392000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=1.592000
duration_time=0.210000
[/PACKET]
[PACKET]
dts_time=1.802000
duration_time=0.174000
[/PACKET]
[PACKET]
dts_time=1.976000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=2.176000
duration_time=0.404000
[/PACKET]
[PACKET]
dts_time=2.580000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=2.780000
duration_time=0.012000
[/PACKET]
[PACKET]
dts_time=2.792000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=2.992000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=3.192000
duration_time=0.216000
[/PACKET]
[PACKET]
dts_time=3.408000
duration_time=0.190000
[/PACKET]
[PACKET]
dts_time=3.598000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=3.798000
duration_time=0.213000
[/PACKET]
[PACKET]
dts_time=4.011000
duration_time=0.180000
[/PACKET]
[PACKET]
dts_time=4.191000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=4.391000
duration_time=0.243000
[/PACKET]
[PACKET]
dts_time=4.634000
duration_time=0.155000
[/PACKET]
[PACKET]
dts_time=4.789000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=4.989000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=5.189000
duration_time=0.202000
[/PACKET]
[PACKET]
dts_time=5.391000
duration_time=0.225000
[/PACKET]
[PACKET]
dts_time=5.616000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=5.816000
duration_time=0.193000
[/PACKET]
[PACKET]
dts_time=6.009000
duration_time=0.222000
[/PACKET]
[PACKET]
dts_time=6.231000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=6.431000
duration_time=0.153000
[/PACKET]
[PACKET]
dts_time=6.584000
duration_time=0.217000
[/PACKET]
[PACKET]
dts_time=6.801000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=7.001000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=7.201000
duration_time=0.178000
[/PACKET]
[PACKET]
dts_time=7.379000
duration_time=0.210000
[/PACKET]
[PACKET]
dts_time=7.589000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=7.789000
duration_time=0.223000
[/PACKET]
[PACKET]
dts_time=8.012000
duration_time=0.231000
[/PACKET]
[PACKET]
dts_time=8.243000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=8.443000
duration_time=0.200000
[/PACKET]
[PACKET]
dts_time=8.643000
duration_time=0.156000
[/PACKET]
[PACKET]
dts_time=8.799000
duration_time=0.200000
This is the link where I compared all the duration values https://docs.google.com/spreadsheets/d/1kzHXJRCMSMFHstzcJHi8DF_U7UkdPp6IS_aTTL-_noo/edit?usp=sharing did not notice the huge difference between DTS_values more than duration.

Please let me know if I am missing something here , I did not understand why i am not able to play the stream.

Thanks for help :)
you have remuxed your files, it has nothing to do with what hls.js as muxed and is sending via MSE to firefox
With those ts chunks in this link https://drive.google.com/open?id=0BzmeWR8wPIT0VzhpaFVtTFRMZ1E I am able to repro the issue , I combined them into mp4 to see the dts_values variation, would remuxing change the timestamps ?
https://hg.mozilla.org/mozilla-central/rev/61091afacd23
Status: REOPENED → RESOLVED
Closed: 8 years ago8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
(In reply to suhasreddy.1993 from comment #15)
> With those ts chunks in this link
> https://drive.google.com/open?id=0BzmeWR8wPIT0VzhpaFVtTFRMZ1E I am able to
> repro the issue , I combined them into mp4 to see the dts_values variation,
> would remuxing change the timestamps ?

it would.

But in any case, you're probably not using a version with the fix.
that stream above doesn't stall for me anymore.
Streams start showing 10:27:24, plays for 9s (which display 10:27:33 at the time).

FWIW, Safari will stall at 2s
And chrome will only play 8s of it (and the last frame displayed is 10:27:32), it also takes a long time to start (>15s) even with the files being local.

so it wasn't Firefox that has issue with the timestamps generated by hls.js

The ultimate fix would be for hls.js to change how the timestamps are generated.
Comment on attachment 8748060 [details]
MozReview Request: Bug 1269325: [mp4] Recalculate dts after adjusting cts. r?kentuckyfriedtakahe

Approval Request Comment
[Feature/regressing bug #]: 1269325
[User impact if declined]: playback stall.
[Describe test coverage new/current, TreeHerder]: In central, lots of mochitest, manually tested
[Risks and why]: Low. They aren't very common mp4 to start with and the issue only affects MSE. The way to bug was crafted, it can't regress existing working file
[String/UUID change made/needed]: None
Attachment #8748060 - Flags: approval-mozilla-beta?
Attachment #8748060 - Flags: approval-mozilla-aurora?
Verified as fixed by users in https://github.com/dailymotion/hls.js/issues/333
Comment on attachment 8748060 [details]
MozReview Request: Bug 1269325: [mp4] Recalculate dts after adjusting cts. r?kentuckyfriedtakahe

While this is a valid issue, if it isn't such a common scenario, let's uplift only to Aurora and let it ride the trains from there. Aurora48+, Beta47-
Attachment #8748060 - Flags: approval-mozilla-beta?
Attachment #8748060 - Flags: approval-mozilla-beta-
Attachment #8748060 - Flags: approval-mozilla-aurora?
Attachment #8748060 - Flags: approval-mozilla-aurora+
Depends on: 1271847
Ritu, I've been made aware that this bug is what's causing TED videos to not switch to HTML5 with Firefox and instead to force them to use Flash.

Seeing that flash is a nasty beast we must get rid off, it may be worth reconsidering the issue.

Having said that we will want bug 1271847 too.
Flags: needinfo?(rkothari)
(In reply to Jean-Yves Avenard [:jya] from comment #23)
> Ritu, I've been made aware that this bug is what's causing TED videos to not
> switch to HTML5 with Firefox and instead to force them to use Flash.
> 
> Seeing that flash is a nasty beast we must get rid off, it may be worth
> reconsidering the issue.
> 
> Having said that we will want bug 1271847 too.

Ok. I'll take your word for it. :) I've approved bug 1271847 for uplift to Aurora48. Does it also impact Beta47? If yes, please nominate a patch for beta on that bug.
Flags: needinfo?(rkothari)
Comment on attachment 8748060 [details]
MozReview Request: Bug 1269325: [mp4] Recalculate dts after adjusting cts. r?kentuckyfriedtakahe

I've been told this impacts playback of some TED videos to fallback to Flash instead of using an HTML5 player. Let's uplift to Beta.
Attachment #8748060 - Flags: approval-mozilla-beta- → approval-mozilla-beta+
(Whoever backports this to beta: please backport this *together with* bug 1271847's patch, once that bug has beta uplift approval.  Otherwise, by backporting this bug on its own, we'll be (temporarily) introducing bug 1271847 on beta.)
Depends on: 1232313
bug 1232313 needs to be uplifted first before this one can go in.
Blocks: 1272916
No longer blocks: 1278005
Depends on: 1278005
No longer depends on: 1271847
Regressed by: 1271847
You need to log in before you can comment on or make changes to this bug.