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

Support live streams (ie big timestamps), replace float with double #1910

Merged
merged 1 commit into from
Nov 22, 2023
Merged

Support live streams (ie big timestamps), replace float with double #1910

merged 1 commit into from
Nov 22, 2023

Conversation

MichaelSweden
Copy link
Contributor

b/309222250
Live video streams with epoch timestamps was not possible to play. Huge timestamps values was truncated because use of data type float with limited resolution.

Change-Id: I7bed973b56e58bea5cd4740ce1778f32dfe617e2

BACKGROUND
It is problem to play some live video streams in Cobalt with a javascript player. It turned out to be related to timestamp values. Normally at play of a video the timestamp starts at zero and grow. For live streams they don't. As such a video is continuously created. Then the timestamp can be the current clock, i.e. UTC time. In seconds these values will be quite big and in microseconds huge. Cobalt cannot handle these huge values and therefor cannot play such video streams.

DEVELOPMENT
I started to investigate the call path for the Seek function. The Seek value that comes to a starboard integration is not the same as higher up in function call path. I found out that the function ConvertSecondsToTimestamp will disturb the value. It uses a float type for micro seconds. The float type is only 24 bits. I rewrite it.
In javascript "mediaElement.currentTime = 0.0;" will create a call to currentTimeAttributeSetter in v8c_html_media_element.cc (file created during build) and that calls set_current_time in the HTMLMediaElement class.

Call flow for seek:
In javascript assign currentTime a value.
currentTimeAttributeSetter // v8c_html_media_element.cc at build time
HTMLMediaElement::set_current_time()
HTMLMediaElement::Seek()
WebMediaPlayerImpl::Seek()
WebMediaPlayerImpl::OnPipelineSeek()
WebMediaPlayerImpl::Seek()
SbPlayerPipeline::Seek()
SbPlayerBridge::PrepareForSeek()
SbPlayerBridge::Seek()
SbPlayerPrivate::Seek()
PlayerWorker::Seek()
PlayerWorker::DoSeek()
PlayerWorkerHandler::Seek() // third_party/starboard integration

I discover that timestamp is saved in different data types. In some places as seconds in a float. The floating-point data type "float" doesn't have enough resolution to handle timestamps. For example date of today will be approx. 1,695,316,864. A float mantissa is only 24 bits. In javascript all numbers are 64 bit float (52 bits mantissa, 11 bits exponent, one sign bit), max 9007199254740991 and min -9007199254740991. Let look at a today time in microseconds: 1,695,392,081,110,344 and compare with 9,007,199,254,740,991. It will fit. In C/C++ data type "double" is 64 bit floating-point.

Fortunately, I didn't have to change the glue file between javascript and C++ (v8c_html_media_element.cc). It is already for double. One problem was that the called function in cobalt had a float argument.

I ended up with changing float to double in a lot of places.

While convert from floating-point value to an integer value (e.g. double to int64_t). The value needs to be rounded, not truncated. This is done in function ConvertSecondsToTimestamp. It might be nicer to use the TimeDelta::FromDouble function. However this function will truncate the value and it is a unittest for it. I don't see the use of such function. Well, I didn't change it. It would have effects a lot.

TEST
I have built (debug/devel/gold linux-x64x11) and run unittest (devel). Got 3 failed unittest, nplb about ipv6. I assume this is not caused by me. Executed Cobalt (devel and debug) in Ubuntu before and after change. With a html page running a javascript DASH player. Unfortunately, I have not found any open test streams on internet with huge timestamps. I have used streams from two different sources (with huge timestamps) to validate this change.

RESULT
Now it is possible to play live streams that have timestamps as current time expressed in epoch.

…uble

b/309222250
Live video streams with epoch timestamps was not possible to play. Huge timestamps values was truncated because use of data type float with limited resolution.

Change-Id: I7bed973b56e58bea5cd4740ce1778f32dfe617e2
@MichaelSweden
Copy link
Contributor Author

Many jobs/workflows fails. However it seems to me like it is some system problem, rather than my code changes that cause the failures. For example like this:
"The process '/usr/bin/git' failed with exit code 1"
"This request was automatically failed because there were no enabled runners online to process the request for more than 1 days."

Is it something I have done wrong?

Copy link
Contributor

@sideb0ard sideb0ard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks great, to me, thanks very much!

I'll leave it to Jason to approve however, as I'm not sure if there are specific platform constraints that would require use of floats to save memory, and/or if there are any wider implications or use of ConvertSecondsToTimestamp.

cobalt/dom/html_media_element.cc Show resolved Hide resolved
Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Attention: 49 lines in your changes are missing coverage. Please review.

Comparison is base (75d03e2) 57.77% compared to head (c44dd03) 57.77%.
Report is 72 commits behind head on main.

Files Patch % Lines
cobalt/dom/html_media_element.cc 0.00% 32 Missing ⚠️
cobalt/media/player/web_media_player_impl.cc 0.00% 16 Missing ⚠️
cobalt/media_session/media_session_client.cc 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #1910    +/-   ##
========================================
  Coverage   57.77%   57.77%            
========================================
  Files        1915     1914     -1     
  Lines       95118    95001   -117     
========================================
- Hits        54950    54890    -60     
+ Misses      40168    40111    -57     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@jasonzhangxx jasonzhangxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, too.

@sideb0ard sideb0ard merged commit ee2f99f into youtube:main Nov 22, 2023
329 of 331 checks passed
@sideb0ard
Copy link
Contributor

thanks Michael, much appreciated!

@MichaelSweden
Copy link
Contributor Author

You are welcome.

How about the nightly jobs. They still try to run and fails. I guess they cannot run because I'm an outside collaborator. I guess someone need to give them access rights to be able to run. Still now after merge they try to run in the night and I get several emails.

@MichaelSweden MichaelSweden deleted the fix-live-streams branch November 28, 2023 08:14
Rongo-JL pushed a commit to Rongo-JL/cobalt that referenced this pull request Dec 19, 2023
…outube#1910)

b/309222250
Live video streams with epoch timestamps was not possible to play. Huge
timestamps values was truncated because use of data type float with
limited resolution.

Change-Id: I7bed973b56e58bea5cd4740ce1778f32dfe617e2

**BACKGROUND**
It is problem to play some live video streams in Cobalt with a
javascript player. It turned out to be related to timestamp values.
Normally at play of a video the timestamp starts at zero and grow. For
live streams they don't. As such a video is continuously created. Then
the timestamp can be the current clock, i.e. UTC time. In seconds these
values will be quite big and in microseconds huge. Cobalt cannot handle
these huge values and therefor cannot play such video streams.

**DEVELOPMENT**
I started to investigate the call path for the Seek function. The Seek
value that comes to a starboard integration is not the same as higher up
in function call path. I found out that the function
ConvertSecondsToTimestamp will disturb the value. It uses a float type
for micro seconds. The float type is only 24 bits. I rewrite it.
In javascript "mediaElement.currentTime = 0.0;" will create a call to
currentTimeAttributeSetter in v8c_html_media_element.cc (file created
during build) and that calls set_current_time in the HTMLMediaElement
class.

Call flow for seek:
In javascript assign currentTime a value.
currentTimeAttributeSetter // v8c_html_media_element.cc at build time
HTMLMediaElement::set_current_time()
HTMLMediaElement::Seek()
WebMediaPlayerImpl::Seek()
WebMediaPlayerImpl::OnPipelineSeek()
WebMediaPlayerImpl::Seek()
SbPlayerPipeline::Seek()
SbPlayerBridge::PrepareForSeek()
SbPlayerBridge::Seek()
SbPlayerPrivate::Seek()
PlayerWorker::Seek()
PlayerWorker::DoSeek()
PlayerWorkerHandler::Seek() // third_party/starboard integration

I discover that timestamp is saved in different data types. In some
places as seconds in a float. The floating-point data type "float"
doesn't have enough resolution to handle timestamps. For example date of
today will be approx. 1,695,316,864. A float mantissa is only 24 bits.
In javascript all numbers are 64 bit float (52 bits mantissa, 11 bits
exponent, one sign bit), max 9007199254740991 and min -9007199254740991.
Let look at a today time in microseconds: 1,695,392,081,110,344 and
compare with 9,007,199,254,740,991. It will fit. In C/C++ data type
"double" is 64 bit floating-point.

Fortunately, I didn't have to change the glue file between javascript
and C++ (v8c_html_media_element.cc). It is already for double. One
problem was that the called function in cobalt had a float argument.

I ended up with changing float to double in a lot of places.

While convert from floating-point value to an integer value (e.g. double
to int64_t). The value needs to be rounded, not truncated. This is done
in function ConvertSecondsToTimestamp. It might be nicer to use the
TimeDelta::FromDouble function. However this function will truncate the
value and it is a unittest for it. I don't see the use of such function.
Well, I didn't change it. It would have effects a lot.

**TEST**
I have built (debug/devel/gold linux-x64x11) and run unittest (devel).
Got 3 failed unittest, nplb about ipv6. I assume this is not caused by
me. Executed Cobalt (devel and debug) in Ubuntu before and after change.
With a html page running a javascript DASH player. Unfortunately, I have
not found any open test streams on internet with huge timestamps. I have
used streams from two different sources (with huge timestamps) to
validate this change.

**RESULT**
Now it is possible to play live streams that have timestamps as current
time expressed in epoch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants