Skip to content

Comments

Handle buffer offset in servosrc#239

Merged
bors-servo merged 4 commits intoservo:masterfrom
ceyusa:offset-handling
Apr 19, 2019
Merged

Handle buffer offset in servosrc#239
bors-servo merged 4 commits intoservo:masterfrom
ceyusa:offset-handling

Conversation

@ceyusa
Copy link
Contributor

@ceyusa ceyusa commented Apr 18, 2019

Seeking requires a strict handling of each buffer offset. Though appsrc do some offset handling, but only for live stream (not seeking). When random-access is enabled the application must take care of marking each buffer offset correctly.

This patch extends servosrc's method push_buffer() so it will count the current offset and stamps it on the pushed buffer. Also, it honors the buffer's block size defined in appsrc, splitting the input data in several buffers if it's bigger, fixing some problems with the buffering mechanism in playbin.

When a seek operation is done, the current media offset must be updated with the requested position.

As per media/player design, it is required that the user must push synchronized the data from the requested position by the seek.

@ceyusa
Copy link
Contributor Author

ceyusa commented Apr 18, 2019

@ferjm r?

ceyusa added 3 commits April 19, 2019 13:54
Seeking requires a strict handling of each buffer offset. Though
appsrc do some offset handling, but only for live stream (not
seeking). When random-access is enabled the application must take care
of marking each buffer offset correctly.

This patch extends servosrc's method push_buffer() so it will count
the current offset and stamps it on the pushed buffer. Also, it honors
the buffer's block size defined in appsrc, splitting the input data in
several buffers if it's bigger, fixing some problems with the
buffering mechanism in playbin.

When a seek operation is done, the current media offset must be
updated with the requested position.

As per media/player design, it is required that the user must push
synchronized the data from the requested position by the seek.
Rather that "guess" when the source element has enough data in its
adapter, let's just use the its signal to semaphore when to fail
push_data() method.
Copy link
Contributor

@ferjm ferjm left a comment

Choose a reason for hiding this comment

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

Looks great. Thanks! r=me

.push_data(Vec::from(&buffer[0..size]))
{
println!("Can't push data: {:?}", e);
//break;
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually it is a hack left behind. Let bring back the break if it is not a enough data.

@ceyusa
Copy link
Contributor Author

ceyusa commented Apr 19, 2019

@bors-servo r=ferjm

@bors-servo
Copy link
Contributor

📌 Commit b393c7f has been approved by ferjm

@bors-servo
Copy link
Contributor

⌛ Testing commit b393c7f with merge 61630b3...

bors-servo pushed a commit that referenced this pull request Apr 19, 2019
Handle buffer offset in servosrc

Seeking requires a strict handling of each buffer offset. Though appsrc do some offset handling, but only for live stream (not seeking). When random-access is enabled the application must take care of marking each buffer offset correctly.

This patch extends servosrc's method push_buffer() so it will count the current offset and stamps it on the pushed buffer. Also, it honors the buffer's block size defined in appsrc, splitting the input data in several buffers if it's bigger, fixing some problems with the buffering mechanism in playbin.

When a seek operation is done, the current media offset must be updated with the requested position.

As per media/player design, it is required that the user must push synchronized the data from the requested position by the seek.
@bors-servo
Copy link
Contributor

☀️ Test successful - checks-travis
Approved by: ferjm
Pushing 61630b3 to master...

@bors-servo bors-servo merged commit b393c7f into servo:master Apr 19, 2019
@ceyusa ceyusa deleted the offset-handling branch April 19, 2019 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants