shaka-project / shaka-packager

A media packaging and development framework for VOD and Live DASH and HLS applications, supporting Common Encryption for Widevine and other DRM Systems.
https://shaka-project.github.io/shaka-packager/
Other
1.9k stars 497 forks source link

insufficient segment references when used with ad_cues #742

Open komar007 opened 4 years ago

komar007 commented 4 years ago

System info

Operating System: docker on gentoo Shaka Packager Version: v2.4.2-c60e988afa-release

Issue and steps to reproduce the problem

Packager Command:

docker run --rm -t -i -v ~/temp/shaka-packager_ac_cues_issue:/temp google/shaka-packager \
packager --segment_duration 2 \
         --ad_cues 2 \
         --generate_static_live_mpd \
         --mpd_output /temp/out/manifest.mpd \
     'input=/temp/video.mp4,stream_selector=video,init_segment=/temp/out/v_init.mp4,segment_template=/temp/out/v_$Number$.m4s' \
         'input=/temp/audio.mp4,stream_selector=audio,init_segment=/temp/out/a_init.mp4,segment_template=/temp/out/a_$Number$.m4s'

Extra steps to reproduce the problem? unpack attached zip file in ~/temp/ before running command

What is the expected result? the AdaptationSet for audio in Period@id=0 contains segment references to both generated audio segments, because the length of the first segment = 88064/44100 < 2s and period's duration is 2s, the remaining 3ms at the end of the period should be covered by the first 3ms of the second segment.

What happens instead? the AdaptationSet for audio in Period@id=0 contains only one segment reference to a segment whose duration is 88064/44100 < 2s, which violates 9.2.1, figure 8, third drawing of https://dashif-documents.azurewebsites.net/Guidelines-TimingModel/master/Guidelines-TimingModel.html#necessary-references-static

shaka-packager_ac_cues_issue.zip

kqyang commented 4 years ago

@komar007 Thanks for bringing this up.

It seems like the only way to be compatible to this specification is to repeat the segment at the period boundary in both periods as in section 12.1: https://dashif-documents.azurewebsites.net/Guidelines-TimingModel/master/Guidelines-TimingModel.html#connectivity-duplicates.

It requires some extra work in the players to detect and de-duplicate the shared segment to avoid presenting it twice. @joeyparrish Is it supported in Shaka Player today?

On the other hand, for AAC audio stream with a sampling rate of 44100, the audio sample size is 1024 / 44100 = ~23ms, which is much larger than the 3ms gap. Can we consider the segment and the period as more or less aligned?

komar007 commented 4 years ago

@kqyang

I just tested that Shaka Player can play a SegmentTemplate+SegmentTimeline stream with duplicated segments correctly. Note that in case of streams that use SegmentBase addressing, segment duplication is unavoidable, because the (implicit) SegmentTimeline for all periods may be the same (they may all use the same sidx). Shaka Packager kind of implicitly generates conforming streams when it's configured to generate SegmentBase-addressed streams. This case also works in Shaka Player.

kqyang commented 4 years ago

@komar007 Thanks for the confirmation! Have you tested dash.js and exo-player too?

Shaka Packager kind of implicitly generates conforming streams when it's configured to generate SegmentBase-addressed streams.

Yes, we actually had some discussions on what is the right behavior in https://github.com/Dash-Industry-Forum/DASH-IF-IOP/issues/166. The players may have special handlings to deal with that.

I am not sure if the players support repeated segments too with SegmentTemplate + SegmentTimeline.

komar007 commented 4 years ago

Yes, we actually had some discussions on what is the right behavior in Dash-Industry-Forum/DASH-IF-IOP#166. The players may have special handlings to deal with that.

Interesting read, thanks for that.

I have tested dash.js 3.0.3 and it gets a strange hiccup between period 2 and 3, where segments are not duplicated, after fluently transitioning between periods 1 and 2, where a chunk is duplicated (I have a 5-period clip with periods of lengths 10, 10, 20, 10 and 10 without segment duplication and with period-continuity signaled, where I manually duplicate entries).

Anyway, that's what I know now. It's a hard decision, where there is a number of implementations working fine and potentially not ready for spec-compliant input. I know something about it:)

kqyang commented 4 years ago

@komar007 Thanks for checking. I think one option we have is to support DASH timing model under a flag, --generate_dash_timing_model_compliant_mpd, default to false for now. We can enable it by default once it is supported by all major players.

@komar007 Will you be interested in helping us implement the fix?

Here are the changes we need:

(1) We can keep the main chunking logic as is for now. For audio, right now we determine whether a sample belongs to the current period or the next period by comparing the average of sample start and sample end with the cue timestamp. We should change it to the sample start: https://github.com/google/shaka-packager/blob/master/packager/media/chunking/cue_alignment_handler.cc#L40.

(2) CopyRepresentation() needs to take an additional parameter for the period start timestamp, which will be used to determine whether the last segment should be included in the new Representation: https://github.com/google/shaka-packager/blob/master/packager/mpd/base/simple_mpd_notifier.cc#L128.

As a long term improvement, we may change the chunking logic to remove the cue point dependency as we do not need to cut the segments at the cue point.

komar007 commented 4 years ago

I haven't had much time to look into it, unfortunately. A while ago I got down to this issue, but I found that for some reason shaka-packager stopped building in docker (I cannot even get the Dockerfile from the git repo to build). I'll try to isolate the problem and file another issue, not sure where the problem is. Could be shaka-packager, depot_tools or alpine linux.

cosmin commented 1 month ago

@komar007 fwiw the build issues should be fixed now and it should be much easier to build the project without reliance on depot tools..

komar007 commented 3 weeks ago

Thanks, @cosmin. I'll try next time I have the opportunity. Unfortunately I am not working with shaka-packager right now, and my container won't build anymore, so I can't quickly verify.