Closed Bug 1283020 Opened 4 years ago Closed 4 years ago

Change the various latency API to use frames instead of milliseconds

Categories

(Core :: Audio/Video: cubeb, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox50 --- fixed

People

(Reporter: padenot, Assigned: padenot)

References

Details

Attachments

(1 file)

The latency situation has evolved a lot since the early days, and now we need a tighter control over the latency numbers, both when getting the minimum latency supported on the current configuration and passing it when initializing a new stream.

Platform APIs usually use frames, and using milliseconds in Gecko forces conversion and unnecessary rounding.

Considering that having the wrong buffer size can make us not go into a mixer fast-path (for example, android is very picky about that), we should switch to using frames in:

- cubeb_device_info
- cubeb_get_min_latency
- cubeb_stream_init
Assignee: nobody → padenot
Rank: 25
Priority: -- → P2
Depends on: 1285541
Comment on attachment 8772781 [details]
Bug 1283020 - Update Gecko to reflect cubeb changes: use frames instead of milliseconds for latency.

https://reviewboard.mozilla.org/r/65518/#review62504

Looks good, thanks!

::: dom/media/AudioStream.cpp:366
(Diff revision 1)
>      return NS_ERROR_FAILURE;
>    }
>  
>    cubeb_stream* stream = nullptr;
> +  /* Convert from milliseconds to frames. */
> +  uint32_t latency_frames = CubebUtils::GetCubebLatency() * aParams.rate / 1000;

We could add the trsform from ms to frames inside CubebUtils::GetCubebLatency method
Attachment #8772781 - Flags: review?(achronop) → review+
Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/mozilla-inbound/rev/344300260f4c
Update cubeb consumers to pass in latency in frames and not in ms.  r=achronop
Pushed by paul@paul.cx:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7ba223fde0e0
Update cubeb consumers to pass in latency in frames and not in ms.  r=achronop
Status: NEW → RESOLVED
Closed: 4 years ago
Flags: needinfo?(padenot)
Resolution: --- → FIXED
oops, still not merged.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://hg.mozilla.org/mozilla-central/rev/7ba223fde0e0
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.