various doc improvements

I bumped the minimum Rust version because I'm taking advantage of
the rustdoc linking added in Rust 1.48:
https://blog.rust-lang.org/2020/11/19/Rust-1.48.html#easier-linking-in-rustdoc

.github/workflows/ci.yml

@@ -13,7 +13,7 @@ jobs:
       matrix:
         rust:
           - stable
-          - 1.45.0
+          - 1.48.0
           - nightly
         include:
           - rust: nightly

guide/build.md

@@ -190,7 +190,7 @@ following command:
 $ brew install ffmpeg node
 ```
 
-Next, you need Rust 1.45+ and Cargo. The easiest way to install them is by
+Next, you need Rust 1.48+ and Cargo. The easiest way to install them is by
 following the instructions at [rustup.rs](https://www.rustup.rs/).
 
 Once prerequisites are installed, you can build the server and find it in

server/db/auth.rs

@@ -2,6 +2,8 @@
 // Copyright (C) 2018 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
+//! Authentication schema: users and sessions/cookies.
+
 use crate::schema::Permissions;
 use base::{bail_t, format_err_t, strutil, ErrorKind, ResultExt};
 use failure::{bail, format_err, Error};

server/db/compare.rs

@@ -2,6 +2,10 @@
 // Copyright (C) 2020 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
+//! Comparison of actual and expected on-disk schema.
+//! This is used as part of the `moonfire-nvr check` database integrity checking
+//! and for tests of `moonfire-nvr upgrade`.
+
 use failure::Error;
 use prettydiff::diff_slice;
 use rusqlite::params;

server/db/db.rs

@@ -1,5 +1,5 @@
 // This file is part of Moonfire NVR, a security camera network video recorder.
-// Copyright (C) 2020 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
+// Copyright (C) 2021 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
 //! Database access logic for the Moonfire NVR SQLite schema.
@@ -7,22 +7,22 @@
 //! The SQLite schema includes everything except the actual video samples (see the `dir` module
 //! for management of those). See `schema.sql` for a more detailed description.
 //!
-//! The `Database` struct caches data in RAM, making the assumption that only one process is
+//! The [`Database`] struct caches data in RAM, making the assumption that only one process is
 //! accessing the database at a time. Performance and efficiency notes:
 //!
-//!   * several query operations here feature row callbacks. The callback is invoked with
+//! *   several query operations here feature row callbacks. The callback is invoked with
 //!     the database lock. Thus, the callback shouldn't perform long-running operations.
 //!
-//!   * startup may be slow, as it scans the entire index for the recording table. This seems
+//! *   startup may be slow, as it scans the entire index for the recording table. This seems
 //!     acceptable.
 //!
-//!   * the operations used for web file serving should return results with acceptable latency.
+//! *   the operations used for web file serving should return results with acceptable latency.
 //!
-//!   * however, the database lock may be held for longer than is acceptable for
+//! *   however, the database lock may be held for longer than is acceptable for
 //!     the critical path of recording frames. The caller should preallocate sample file uuids
 //!     and such to avoid database operations in these paths.
 //!
-//!   * adding and removing recordings done during normal operations use a batch interface.
+//! *   adding and removing recordings done during normal operations use a batch interface.
 //!     A list of mutations is built up in-memory and occasionally flushed to reduce SSD write
 //!     cycles.
 
@@ -457,7 +457,8 @@ pub struct Stream {
 
 /// Bounds of a live view segment. Currently this is a single frame of video.
 /// This is used for live stream recordings. The stream id should already be known to the
-/// subscriber.
+/// subscriber. Note this doesn't actually contain the video, just a reference that can be
+/// looked up within the database.
 #[derive(Clone, Debug)]
 pub struct LiveSegment {
     pub recording: i32,
@@ -590,6 +591,9 @@ pub struct Open {
     pub(crate) uuid: Uuid,
 }
 
+/// A combination of a stream id and recording id into a single 64-bit int.
+/// This is used as a primary key in the SQLite `recording` table (see `schema.sql`)
+/// and the sample file's name on disk (see `dir.rs`).
 #[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)]
 pub struct CompositeId(pub i64);
 
@@ -2285,6 +2289,7 @@ impl<C: Clocks + Clone> Database<C> {
     }
 }
 
+/// Reference to a locked database returned by [Database::lock].
 pub struct DatabaseGuard<'db, C: Clocks> {
     clocks: &'db C,
     db: MutexGuard<'db, LockedDatabase>,

server/db/dir.rs

@@ -4,7 +4,8 @@
 
 //! Sample file directory management.
 //!
-//! This includes opening files for serving, rotating away old files, and saving new files.
+//! This mostly includes opening a directory and looking for recordings within it.
+//! Updates to the directory happen through [crate::writer].
 
 use crate::coding;
 use crate::db::CompositeId;
@@ -27,22 +28,28 @@ use std::sync::Arc;
 
 /// The fixed length of a directory's `meta` file.
 ///
-/// See DirMeta comments within proto/schema.proto for more explanation.
+/// See `DirMeta` comments within `proto/schema.proto` for more explanation.
 const FIXED_DIR_META_LEN: usize = 512;
 
 /// A sample file directory. Typically one per physical disk drive.
 ///
-/// If the directory is used for writing, the `start_syncer` function should be called to start
-/// a background thread. This thread manages deleting files and writing new files. It synces the
-/// directory and commits these operations to the database in the correct order to maintain the
-/// invariants described in `design/schema.md`.
+/// If the directory is used for writing, [crate::writer::start_syncer] should be
+/// called to start a background thread. This thread manages deleting files and
+/// writing new files. It synces the directory and commits these operations to
+/// the database in the correct order to maintain the invariants described in
+/// `design/schema.md`.
 #[derive(Debug)]
 pub struct SampleFileDir {
-    /// The open file descriptor for the directory. The worker uses it to create files and sync the
-    /// directory. Other threads use it to open sample files for reading during video serving.
+    /// The open file descriptor for the directory. The worker created by
+    /// [crate::writer::start_syncer] uses it to create files and sync the
+    /// directory. Other threads use it to open sample files for reading during
+    /// video serving.
     pub(crate) fd: Fd,
 }
 
+/// The on-disk filename of a recording file within the sample file directory.
+/// This is the [`CompositeId`](crate::db::CompositeId) as 16 hexadigits. It's
+/// null-terminated so it can be passed to system calls without copying.
 pub(crate) struct CompositeIdPath([u8; 17]);
 
 impl CompositeIdPath {
@@ -101,6 +108,7 @@ impl Fd {
         Ok(Fd(fd))
     }
 
+    /// `fsync`s this directory, causing all file metadata to be committed to permanent storage.
     pub(crate) fn sync(&self) -> Result<(), nix::Error> {
         nix::unistd::fsync(self.0)
     }
@@ -110,6 +118,7 @@ impl Fd {
         nix::fcntl::flock(self.0, arg)
     }
 
+    /// Returns information about the filesystem on which this directory lives.
     pub fn statfs(&self) -> Result<nix::sys::statvfs::Statvfs, nix::Error> {
         nix::sys::statvfs::fstatvfs(self)
     }
@@ -147,7 +156,7 @@ pub(crate) fn read_meta(dir: &Fd) -> Result<schema::DirMeta, Error> {
     Ok(meta)
 }
 
-/// Write `dir`'s metadata, clobbering existing data.
+/// Writes `dirfd`'s metadata, clobbering existing data.
 pub(crate) fn write_meta(dirfd: RawFd, meta: &schema::DirMeta) -> Result<(), Error> {
     let mut data = meta
         .write_length_delimited_to_bytes()
@@ -340,7 +349,7 @@ impl SampleFileDir {
 
 /// Parses a composite id filename.
 ///
-/// These are exactly 16 bytes, lowercase hex.
+/// These are exactly 16 bytes, lowercase hex, as created by [CompositeIdPath].
 pub(crate) fn parse_id(id: &[u8]) -> Result<CompositeId, ()> {
     if id.len() != 16 {
         return Err(());

server/db/fs.rs

@@ -2,11 +2,14 @@
 // Copyright (C) 2019 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
+//! Filesystem utilities.
+
 use nix::fcntl::OFlag;
 use nix::sys::stat::Mode;
 use nix::NixPath;
 use std::os::unix::io::{FromRawFd, RawFd};
 
+/// Opens the given `path` within `dirfd` with the specified flags.
 pub fn openat<P: ?Sized + NixPath>(
     dirfd: RawFd,
     path: &P,

server/db/lib.rs

@@ -1,7 +1,16 @@
 // This file is part of Moonfire NVR, a security camera network video recorder.
-// Copyright (C) 2020 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
+// Copyright (C) 2021 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
+//! Moonfire NVR's persistence layer.
+//!
+//! This manages both the SQLite database and the sample file directory.
+//! Everything dealing with either flows through this crate. It keeps in-memory
+//! state both as indexes and to batch SQLite database transactions.
+//!
+//! The core recording design is described in `design/recording.md` and is
+//! mostly in the `db` module.
+
 #![cfg_attr(all(feature = "nightly", test), feature(test))]
 
 pub mod auth;

server/db/proto/schema.proto

@@ -4,13 +4,13 @@
 
 syntax = "proto3";
 
-// Metadata stored in sample file dirs as "<dir>/meta". This is checked
+// Metadata stored in sample file dirs as `<dir>/meta`. This is checked
 // against the metadata stored within the database to detect inconsistencies
 // between the directory and database, such as those described in
-// design/schema.md.
+// `design/schema.md`.
 //
 // As of schema version 4, the overall file format is as follows: a
-// varint-encoded length, followed by a serialized DirMeta message, followed
+// varint-encoded length, followed by a serialized `DirMeta` message, followed
 // by NUL bytes padding to a total length of 512 bytes. This message never
 // exceeds that length.
 //

server/db/recording.rs

@@ -2,6 +2,8 @@
 // Copyright (C) 2020 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
+//! Building and reading recordings via understanding of their sample indexes.
+
 use crate::coding::{append_varint32, decode_varint32, unzigzag32, zigzag32};
 use crate::db;
 use failure::{bail, Error};
@@ -17,7 +19,7 @@ pub const MAX_RECORDING_WALL_DURATION: i64 = 5 * 60 * TIME_UNITS_PER_SEC;
 pub use base::time::Duration;
 pub use base::time::Time;
 
-/// Converts from a wall time offset into a recording to a media time offset or vice versa.
+/// Converts from a wall time offset within a recording to a media time offset or vice versa.
 pub fn rescale(from_off_90k: i32, from_duration_90k: i32, to_duration_90k: i32) -> i32 {
     debug_assert!(
         from_off_90k <= from_duration_90k,
@@ -31,7 +33,7 @@ pub fn rescale(from_off_90k: i32, from_duration_90k: i32, to_duration_90k: i32)
     }
 
     // The intermediate values here may overflow i32, so use an i64 instead. The max wall
-    // time is recording::MAX_RECORDING_WALL_DURATION; the max media duration should be
+    // time is [`MAX_RECORDING_WALL_DURATION`]; the max media duration should be
     // roughly the same (design limit of 500 ppm correction). The final result should fit
     // within i32.
     i32::try_from(
@@ -46,7 +48,7 @@ pub fn rescale(from_off_90k: i32, from_duration_90k: i32, to_duration_90k: i32)
     .unwrap()
 }
 
-/// An iterator through a sample index.
+/// An iterator through a sample index (as described in `design/recording.md`).
 /// Initially invalid; call `next()` before each read.
 #[derive(Clone, Copy, Debug)]
 pub struct SampleIndexIterator {
@@ -144,6 +146,7 @@ impl SampleIndexIterator {
     }
 }
 
+/// An encoder for a sample index (as described in `design/recording.md`).
 #[derive(Debug)]
 pub struct SampleIndexEncoder {
     prev_duration_90k: i32,
@@ -191,9 +194,10 @@ impl SampleIndexEncoder {
     }
 }
 
-/// A segment represents a view of some or all of a single recording, starting from a key frame.
+/// A segment represents a view of some or all of a single recording.
 /// This struct is not specific to a container format; for `.mp4`s, it's wrapped in a
-/// `mp4::Segment`. Other container/transport formats could be supported in a similar manner.
+/// `moonfire_nvr::mp4::Segment`. Other container/transport formats could be
+/// supported in a similar manner.
 #[derive(Debug)]
 pub struct Segment {
     pub id: db::CompositeId,

server/db/schema.sql

@@ -413,7 +413,10 @@ create table user_session (
 
 create index user_session_uid on user_session (user_id);
 
--- Timeseries with an enum value.
+-- Timeseries with an enum value, eg:
+-- *   camera motion detection results (unknown, still, moving)
+-- *   security system arm status (unknown, disarmed, away, stay)
+-- *   security system zone status (unknown, normal, violated, trouble)
 create table signal (
   id integer primary key,
 
@@ -436,8 +439,7 @@ create table signal (
   unique (source_uuid, type_uuid)
 );
 
--- e.g. "moving/still", "disarmed/away/stay", etc.
--- TODO: just do a protobuf for each type? might be simpler, more flexible.
+-- e.g. "still/moving", "disarmed/away/stay", etc.
 create table signal_type_enum (
   type_uuid blob not null check (length(type_uuid) = 16),
   value integer not null check (value > 0 and value < 16),

server/db/signal.rs

@@ -2,6 +2,9 @@
 // Copyright (C) 2019 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
+//! Schema for "signals": enum-valued timeserieses.
+//! See the `signal` table within `schema.sql` for more information.
+
 use crate::db::FromSqlUuid;
 use crate::recording;
 use crate::{coding, days};

server/db/testutil.rs

@@ -2,6 +2,9 @@
 // Copyright (C) 2020 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
+//! Utilities for automated testing involving Moonfire NVR's persistence library.
+//! Used for tests of both the `moonfire_db` crate itself and the `moonfire_nvr` crate.
+
 use crate::db;
 use crate::dir;
 use crate::writer;

server/db/upgrade/mod.rs

@@ -2,9 +2,10 @@
 // Copyright (C) 2020 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
-/// Upgrades the database schema.
-///
-/// See `guide/schema.md` for more information.
+//! Upgrades the database schema.
+//!
+//! See `guide/schema.md` for more information.
+
 use crate::db;
 use failure::{bail, Error};
 use log::info;

server/db/writer.rs

@@ -2,9 +2,7 @@
 // Copyright (C) 2020 The Moonfire NVR Authors; see AUTHORS and LICENSE.txt.
 // SPDX-License-Identifier: GPL-v3.0-or-later WITH GPL-3.0-linking-exception.
 
-//! Sample file directory management.
-//!
-//! This includes opening files for serving, rotating away old files, and saving new files.
+//! Writing recordings and deleting old ones.
 
 use crate::db::{self, CompositeId};
 use crate::dir;
@@ -23,6 +21,9 @@ use std::thread;
 use std::time::Duration as StdDuration;
 use time::{Duration, Timespec};
 
+/// Trait to allow mocking out [crate::dir::SampleFileDir] in syncer tests.
+/// This is public because it's exposed in the [SyncerChannel] type parameters,
+/// not because it's of direct use outside this module.
 pub trait DirWriter: 'static + Send {
     type File: FileWriter;
 
@@ -31,6 +32,9 @@ pub trait DirWriter: 'static + Send {
     fn unlink_file(&self, id: CompositeId) -> Result<(), nix::Error>;
 }
 
+/// Trait to allow mocking out [std::fs::File] in syncer tests.
+/// This is public because it's exposed in the [SyncerChannel] type parameters,
+/// not because it's of direct use outside this module.
 pub trait FileWriter: 'static {
     /// As in `std::fs::File::sync_all`.
     fn sync_all(&self) -> Result<(), io::Error>;
@@ -62,10 +66,16 @@ impl FileWriter for ::std::fs::File {
     }
 }
 
-/// A command sent to the syncer. These correspond to methods in the `SyncerChannel` struct.
+/// A command sent to a [Syncer].
 enum SyncerCommand<F> {
+    /// Command sent by [SyncerChannel::async_save_recording].
     AsyncSaveRecording(CompositeId, recording::Duration, F),
+
+    /// Notes that the database has been flushed and garbage collection should be attempted.
+    /// [start_syncer] sets up a database callback to send this command.
     DatabaseFlushed,
+
+    /// Command sent by [SyncerChannel::flush].
     Flush(mpsc::SyncSender<()>),
 }
 
@@ -79,7 +89,7 @@ impl<F> ::std::clone::Clone for SyncerChannel<F> {
     }
 }
 
-/// State of the worker thread.
+/// State of the worker thread created by [start_syncer].
 struct Syncer<C: Clocks + Clone, D: DirWriter> {
     dir_id: i32,
     dir: D,
@@ -87,6 +97,7 @@ struct Syncer<C: Clocks + Clone, D: DirWriter> {
     planned_flushes: std::collections::BinaryHeap<PlannedFlush>,
 }
 
+/// A plan to flush at a given instant due to a recently-saved recording's `flush_if_sec` parameter.
 struct PlannedFlush {
     /// Monotonic time at which this flush should happen.
     when: Timespec,
@@ -99,7 +110,7 @@ struct PlannedFlush {
     reason: String,
 
     /// Senders to drop when this time is reached. This is for test instrumentation; see
-    /// `SyncerChannel::flush`.
+    /// [SyncerChannel::flush].
     senders: Vec<mpsc::SyncSender<()>>,
 }
 
@@ -170,14 +181,18 @@ where
     ))
 }
 
+/// A new retention limit for use in [lower_retention].
 pub struct NewLimit {
     pub stream_id: i32,
     pub limit: i64,
 }
 
-/// Deletes recordings if necessary to fit within the given new `retain_bytes` limit.
+/// Immediately deletes recordings if necessary to fit within the given new `retain_bytes` limit.
 /// Note this doesn't change the limit in the database; it only deletes files.
 /// Pass a limit of 0 to delete all recordings associated with a camera.
+///
+/// This is expected to be performed from `moonfire-nvr config` when no syncer is running.
+/// It potentially flushes the database twice (before and after the actual deletion).
 pub fn lower_retention(
     db: Arc<db::Database>,
     dir_id: i32,
@@ -206,7 +221,9 @@ pub fn lower_retention(
     })
 }
 
-/// Deletes recordings to bring a stream's disk usage within bounds.
+/// Enqueues deletion of recordings to bring a stream's disk usage within bounds.
+/// The next flush will mark the recordings as garbage in the SQLite database, and then they can
+/// be deleted from disk.
 fn delete_recordings(
     db: &mut db::LockedDatabase,
     stream_id: i32,
@@ -463,12 +480,10 @@ impl<C: Clocks + Clone, D: DirWriter> Syncer<C, D> {
         });
     }
 
-    /// Saves the given recording and causes rotation to happen. Called from worker thread.
-    ///
-    /// Note that part of rotation is deferred for the next cycle (saved writing or program startup)
-    /// so that there can be only one dir sync and database transaction per save.
-    /// Internal helper for `save`. This is separated out so that the question-mark operator
-    /// can be used in the many error paths.
+    /// Saves the given recording and prompts rotation. Called from worker thread.
+    /// Note that this doesn't flush immediately; SQLite transactions are batched to lower SSD
+    /// wear. On the next flush, the old recordings will actually be marked as garbage in the
+    /// database, and shortly afterward actually deleted from disk.
     fn save(&mut self, id: CompositeId, wall_duration: recording::Duration, f: D::File) {
         trace!("Processing save for {}", id);
         let stream_id = id.stream();
@@ -583,9 +598,9 @@ enum WriterState<F: FileWriter> {
     Closed(PreviousWriter),
 }
 
-/// State for writing a single recording, used within `Writer`.
+/// State for writing a single recording, used within [Writer].
 ///
-/// Note that the recording created by every `InnerWriter` must be written to the `SyncerChannel`
+/// Note that the recording created by every `InnerWriter` must be written to the [SyncerChannel]
 /// with at least one sample. The sample may have zero duration.
 struct InnerWriter<F: FileWriter> {
     f: F,
@@ -612,6 +627,9 @@ struct InnerWriter<F: FileWriter> {
     unindexed_sample: Option<UnindexedSample>,
 }
 
+/// A sample which has been written to disk but not included in the index yet.
+/// The index includes the sample's duration, which is calculated from the
+/// _following_ sample's pts, so the most recent sample is always unindexed.
 #[derive(Copy, Clone)]
 struct UnindexedSample {
     local_time: recording::Time,
@@ -620,7 +638,7 @@ struct UnindexedSample {
     is_key: bool,
 }
 
-/// State associated with a run's previous recording; used within `Writer`.
+/// State associated with a run's previous recording; used within [Writer].
 #[derive(Copy, Clone)]
 struct PreviousWriter {
     end: recording::Time,

server/src/cmds/ts.rs

@@ -10,9 +10,9 @@ pub struct Args {
     /// Timestamp(s) to translate.
     ///
     /// May be either an integer or an RFC-3339-like string:
-    /// YYYY-mm-dd[THH:MM[:SS[:FFFFF]]][{Z,{+,-,}HH:MM}].
+    /// `YYYY-mm-dd[THH:MM[:SS[:FFFFF]]][{Z,{+,-,}HH:MM}]`.
     ///
-    /// Eg: 142913484000000, 2020-04-26, 2020-04-26T12:00:00:00000-07:00.
+    /// Eg: `142913484000000`, `2020-04-26`, `2020-04-26T12:00:00:00000-07:00`.
     #[structopt(required = true)]
     timestamps: Vec<String>,
 }

server/src/streamer.rs

@@ -27,6 +27,8 @@ where
     pub shutdown: &'b Arc<AtomicBool>,
 }
 
+/// Connects to a given RTSP stream and writes recordings to the database via [`writer::Writer`].
+/// Streamer is meant to be long-lived; it will sleep and retry after each failure.
 pub struct Streamer<'a, C, S>
 where
     C: Clocks + Clone,