I'm not sure what changed. We have a
`Cargo.lock`, and these runs were with identical code:
https://github.com/scottlamb/retina/actions/runs/3148795378 passed
https://github.com/scottlamb/retina/actions/runs/3148801810 failed
Nonetheless, the failures started happening on CI (and not my local
machine). Something changed with respect to the other futures are polled
or something. Not having a keepalive future prevents tokio from
"auto-advancing" the paused time (a misfeature IMHO). There's still
some advance I think from background_teardown's timeout_at, but that's late
enough to not cause the same test failure.
Failure below.
```
---- client::tests::simple stdout ----
thread 'client::tests::simple' panicked at 'assertion failed: `(left == right)`
left: `Options`,
right: `Teardown`', src/client/mod.rs:2547:17
stack backtrace:
0: rust_begin_unwind
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/panicking.rs:584:5
1: core::panicking::panic_fmt
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/panicking.rs:142:14
error: test failed, to rerun pass '-p retina --lib'
2: core::panicking::assert_failed_inner
3: core::panicking::assert_failed
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/panicking.rs:181:5
4: retina::client::tests::req_response::{{closure}}
at ./src/client/mod.rs:2547:17
5: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/future/mod.rs:91:19
6: <tokio::future::maybe_done::MaybeDone<Fut> as core::future::future::Future>::poll
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/future/maybe_done.rs:68:48
7: retina::client::tests::simple::{{closure}}::{{closure}}
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/macros/join.rs:116:24
8: <tokio::future::poll_fn::PollFn<F> as core::future::future::Future>::poll
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/future/poll_fn.rs:38:9
9: retina::client::tests::simple::{{closure}}
at ./src/client/mod.rs:2651:9
10: <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/future/mod.rs:91:19
11: <core::pin::Pin<P> as core::future::future::Future>::poll
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/future/future.rs:124:9
12: tokio::runtime::basic_scheduler::CoreGuard::block_on::{{closure}}::{{closure}}::{{closure}}
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/runtime/basic_scheduler.rs:508:48
13: tokio::coop::with_budget::{{closure}}
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/coop.rs:102:9
14: std:🧵:local::LocalKey<T>::try_with
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/thread/local.rs:445:16
15: std:🧵:local::LocalKey<T>::with
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/std/src/thread/local.rs:421:9
16: tokio::coop::with_budget
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/coop.rs:95:5
17: tokio::coop::budget
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/coop.rs:72:5
18: tokio::runtime::basic_scheduler::CoreGuard::block_on::{{closure}}::{{closure}}
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/runtime/basic_scheduler.rs:508:25
19: tokio::runtime::basic_scheduler::Context::enter
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/runtime/basic_scheduler.rs:362:19
20: tokio::runtime::basic_scheduler::CoreGuard::block_on::{{closure}}
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/runtime/basic_scheduler.rs:507:36
21: tokio::runtime::basic_scheduler::CoreGuard::enter::{{closure}}
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/runtime/basic_scheduler.rs:565:57
22: tokio::macros::scoped_tls::ScopedKey<T>::set
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/macros/scoped_tls.rs:61:9
23: tokio::runtime::basic_scheduler::CoreGuard::enter
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/runtime/basic_scheduler.rs:565:27
24: tokio::runtime::basic_scheduler::CoreGuard::block_on
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/runtime/basic_scheduler.rs:498:9
25: tokio::runtime::basic_scheduler::BasicScheduler::block_on
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/runtime/basic_scheduler.rs:174:24
26: tokio::runtime::Runtime::block_on
at /home/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-1.19.2/src/runtime/mod.rs:480:46
27: retina::client::tests::simple
at ./src/client/mod.rs:2664:9
28: retina::client::tests::simple::{{closure}}
at ./src/client/mod.rs:2561:11
29: core::ops::function::FnOnce::call_once
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5
30: core::ops::function::FnOnce::call_once
at /rustc/a55dd71d5fb0ec5a6a3a9e8c27b2127ba491ce52/library/core/src/ops/function.rs:248:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
```
* match a couple h264-reader semantic changes. E.g., decode_nal now
expects the full NALU including header byte.
* adjust test data in depacketize_parameter_change. The SDP parameters
I used were invalid in a way caught only by the new h264-reader crate.
That's not the intended scenario, so use different parameters. But
mention the accidental scenario in CHANGELOG.md.
* initialize logging in h264 tests to ease debugging problems like the
bullet above.
* in AAC code, match h264-reader's switch from bitreader to
bitstream-io. This keeps dependencies down, in addition to the reasons
for h264-reader's switch, described here:
7a02a3b7bd
The caller gets this information and may do its own logging with more
context. But...sometimes the caller doesn't, and being able to turn
this on is handy then, so log at low priority. (debug; I considered
trace even.)
Part of #47
The main benefit is that `VideoFrame::into_data` can cheaply return a
`Vec<u8>` that the caller can mutate. In particular, they could convert
H.264 data from Annex B to AVC form or skip non-VCL NALs. Neither of
these transformations add bytes so they're both possible without
allocation.
Audio and message frames still use `Bytes` internally, as that allows
them to avoid copying when the frame is wholly contained in a packet.
I think this is much more common than for video. I didn't add an
`AudioFrame::into_data` or `VideoFrame::into_data`.
* and take advantage of it to use `Box<str>` rather than `String`
in a couple places (saving a couple words)
* also rename clock_rate to clock_rate_hz for clarity
As described in #47 and #58: revamp this type to keep the full raw
packet, and provide accessors rather than raw fields. It's also
smaller now, which is nice.
As listed in #47.
This improves throughput in the client/h264 benchmark by about 20%,
I think because we copy fewer bytes around. CodecItem went from 256
bytes to 176.
This was accidentally looking at the bottom bit of the V=2 field, so
it effectively never subtracted padding from the payload.
This bug didn't have any effect...yet. We currently only support
parsing the sender report. RFC 3550 says that SRs are sent
in "compound packets" that start with the SR and have at least one
other entry (a CNAME). Only the last packet within the compound packet
can have padding, so it's never come up in SR parsing. Fix this bug now
before we start parsing more things and run into trouble.
