Content-Length: 259557 | pFad | https://github.com/google/ExoPlayer/issues/5490

A7 EventMessageEncoder/Decoder doesn't work well with Long values · Issue #5490 · google/ExoPlayer · GitHub
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.

Sign up for GitHub

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

Jump to bottom

EventMessageEncoder/Decoder doesn't work well with Long values #5490

Closed
jankowskib opened this issue Feb 9, 2019 · 5 comments
Closed

EventMessageEncoder/Decoder doesn't work well with Long values #5490

jankowskib opened this issue Feb 9, 2019 · 5 comments
Assignees
@ojw28
Labels
bug

Comments

@jankowskib
Copy link

Issue description

I have EventMessages with a large presentation time value e.g. '<Event presentationTime="16122561404240"'. Unfortunately EventMessageEncoder stores this big values using writeUnsignedInt instead of writeUnsignedLong and the part of its value is discarded. So when EventMessageDecoder tries to decode message from inputBuffer the value is totally incorrect.

Reproduction steps

  1. Init ExoPlayer with https://gist.githubusercontent.com/jankowskib/92a5e955d2bf94b45721f4783ec22206/raw/738dc7443071cd8c5e5bf880ecb8ca2666bff02f/scte35.mpd
  2. Set breakpoint on onMetaData
  3. presentationTimeUs value is incorrect

Link to test content

Provide a link to media that reproduces the issue. If you don't wish to post it
publicly, please submit the issue, then email the link to
dev.exoplayer@gmail.com using a subject in the format "Issue #1234".

Version of ExoPlayer being used

2.9.1
@ojw28 ojw28 self-assigned this Feb 10, 2019
@ojw28 ojw28 added the bug label Feb 10, 2019
@ojw28
Copy link
Contributor

ojw28 commented Feb 11, 2019

We're currently misusing the presentation_time_delta Event field to store absolute presentation timestamps. This doesn't work properly for large timestamps, because the field is only 32 bits. We don't actually need to pass the presentation timestamps through the binary representation of EventMessage, so we'll push a fix that stops doing this.

@jankowskib
Copy link
Author

Thank you for quick reply. I temporarily extended serialized field to 64 bit and it works fine. Here's the commit if interested (jankowskib@8d02b75)
@ojw28 Do you suggest to add presentationTime as an argument to EventMessageEncoder.decode(...) in the same way as timescale? I could do a PR with such fix

@ojw28
Copy link
Contributor

ojw28 commented Feb 11, 2019

Extending the serialized field to 64 bit will break parsing of in-band (i.e. embedded in FMP4) emsg atoms. Which may or may not be a problem for your use case. There's no need for EventMessage to contain presentation timestamp at all. We will push a change to fix this problem sometime today or tomorrow.

@jankowskib
Copy link
Author

OK. It's no longer issue for me

@ojw28 ojw28 reopened this Feb 18, 2019
@ojw28
Copy link
Contributor

ojw28 commented Feb 18, 2019

Reopening to track landing a fix. It's still a bug ;).

andrewlewis pushed a commit that referenced this issue Feb 18, 2019
@ojw28 @andrewlewis
Fix EventMessage timestamp issue
c66d26b 
Stop encoding/decoding presentation time as part of the message.
What's actually in emsg boxes is a presentation time delta,
which is why it's only 32 bits, and hence why it doesn't handle
large absolute timestamps. We were using this field to hold
absolute timestamps only for the purpose of passing presentation
times from DashManifestParser.parseEvent back to the calling
method. After this change, we return Pair<Long, EventMessage>
instead.

Issue: #5490
PiperOrigin-RevId: 233561731
andrewlewis pushed a commit that referenced this issue Feb 19, 2019
@ojw28 @andrewlewis
Fix EventMessage timestamp issue
90c4cd2 
Stop encoding/decoding presentation time as part of the message.
What's actually in emsg boxes is a presentation time delta,
which is why it's only 32 bits, and hence why it doesn't handle
large absolute timestamps. We were using this field to hold
absolute timestamps only for the purpose of passing presentation
times from DashManifestParser.parseEvent back to the calling
method. After this change, we return Pair<Long, EventMessage>
instead.

Issue: #5490
PiperOrigin-RevId: 233561731
@ojw28 ojw28 closed this as completed Mar 6, 2019
@google google locked and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@ojw28 ojw28

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jankowskib @ojw28








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/google/ExoPlayer/issues/5490

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy