Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EventMessageEncoder encoding wrong duration #9123

Closed
Reevn opened this issue Jun 27, 2021 · 1 comment
Closed

EventMessageEncoder encoding wrong duration #9123

Reevn opened this issue Jun 27, 2021 · 1 comment
Assignees
Labels

Comments

@Reevn
Copy link

Reevn commented Jun 27, 2021

Hi there!

This problem is not specific to any platform or ExoPlayer version.

Problem

I've encountered a problem with how the duration of EventMessages (such as DASH EventStream events) is parsed. Per DASH spec it is valid to omit the duration of an MPD event, which means that the duration is unknown. In that case, this is the current workflow as far as I understand it:

This leads to the following values:

// current behaviour
decodedEventMessage.durationMs // 2481536660

// behaviour I would expect
decodedEventMessage.durationMs // C.TIME_UNSET

For emsg boxes inside the media container the duration is already encoded, but the decoding step does not check if the encoded duration is the value for "unknown duration".

Proposal

The DASH spec section 5.10.3.3.4 says the following:

event_duration provides the duration of event in media presentation time. The timescale is indicated in the timescale field. The value 0xFFFF indicates an unknown duration.

To me it seems that two things need to be adjusted

  • When parsing the MPD, encode a missing duration in MPD events as 0xFFFF/65536 instead of C_TIME_UNSET
  • When decoding an EventMessage, set EventMessage.durationMs to C_TIME_UNSET if the encoded duration is 0xFFFF/65536

With these two changes both In-band and Out-of-band events would work in the same way and expose an unknown duration as C.TIME_UNSET. I guess this can be seen as a breaking change for In-band events or a bug fix for both In-band and Out-of-band events.

I am happy to open up a PR if the proposal makes sense.

@icbaker
Copy link
Collaborator

icbaker commented Jun 30, 2021

Thanks for reporting this.

EventMessage{De,En}coder use a custom serialization scheme, it's not intended to be compatible with the in-band bytes representation (since 97183ef and a08b537).

So I think the easiest solution is to just encode durationMs as a 64-bit int, instead of an unsigned 32-bit int. I'll make that change (and change id too while I'm at it).

kim-vde pushed a commit that referenced this issue Jul 9, 2021
The serialization scheme used here is custom, it doesn't need
to be compatible with emsg-v0 or emsg-v1 (since
97183ef).

This means that C.TIME_UNSET will propagate correctly through the
serialization.

#minor-release

Issue: #9123
PiperOrigin-RevId: 382762873
@icbaker icbaker closed this as completed Jul 13, 2021
icbaker added a commit that referenced this issue Jul 21, 2021
The serialization scheme used here is custom, it doesn't need
to be compatible with emsg-v0 or emsg-v1 (since
97183ef).

This means that C.TIME_UNSET will propagate correctly through the
serialization.

Issue: #9123
PiperOrigin-RevId: 382762873
@google google locked and limited conversation to collaborators Sep 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants