Skip to content

Commit

Permalink
ui list view: tool tip to see why recording ended
Browse files Browse the repository at this point in the history
Users are often puzzled why there are short recordings. Previously
the only way to see this was to examine Moonfire's logs. This should
be a much better experience to find it right in the UI where you're
wondering, and without the potential the logs are gone.

Fixes #302
  • Loading branch information
scottlamb committed Jun 1, 2024
1 parent adf73a2 commit 0422593
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 15 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ upgrades, e.g. `v0.6.x` -> `v0.7.x`. The config file format and
[API](ref/api.md) currently have no stability guarantees, so they may change
even on minor releases, e.g. `v0.7.5` -> `v0.7.6`.

## unreleased

* in UI's list view, add a tooltip on the end time which shows why the
recording ended.

## v0.7.16 (2024-05-30)

* further changes to improve Reolink camera compatibility.
Expand Down
3 changes: 3 additions & 0 deletions ref/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,9 @@ arbitrary order. Each recording object has the following properties:
and Moonfire NVR fills in a duration of 0. When using `/view.mp4`, it's
not possible to append additional segments after such frames, as noted
below.
* `endReason`: the reason the recording ended. Absent if the recording did
not end (`growing` is true or this was split via `split90k`) or if the
reason was unknown (recording predates schema version 7).

Under the property `videoSampleEntries`, an object mapping ids to objects with
the following properties:
Expand Down
14 changes: 9 additions & 5 deletions server/db/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ impl std::fmt::Debug for VideoSampleEntryToInsert {
}

/// A row used in `list_recordings_by_time` and `list_recordings_by_id`.
#[derive(Copy, Clone, Debug)]
#[derive(Clone, Debug)]
pub struct ListRecordingsRow {
pub start: recording::Time,
pub video_sample_entry_id: i32,
Expand All @@ -200,6 +200,7 @@ pub struct ListRecordingsRow {
/// (It's not included in the `recording_cover` index, so adding it to
/// `list_recordings_by_time` would be inefficient.)
pub prev_media_duration_and_runs: Option<(recording::Duration, i32)>,
pub end_reason: Option<String>,
}

/// A row used in `list_aggregated_recordings`.
Expand All @@ -217,6 +218,7 @@ pub struct ListAggregatedRecordingsRow {
pub first_uncommitted: Option<i32>,
pub growing: bool,
pub has_trailing_zero: bool,
pub end_reason: Option<String>,
}

impl ListAggregatedRecordingsRow {
Expand All @@ -241,6 +243,7 @@ impl ListAggregatedRecordingsRow {
},
growing,
has_trailing_zero: (row.flags & RecordingFlags::TrailingZero as i32) != 0,
end_reason: row.end_reason,
}
}
}
Expand Down Expand Up @@ -301,6 +304,7 @@ impl RecordingToInsert {
open_id,
flags: self.flags | RecordingFlags::Uncommitted as i32,
prev_media_duration_and_runs: Some((self.prev_media_duration, self.prev_runs)),
end_reason: self.end_reason.clone(),
}
}
}
Expand Down Expand Up @@ -1376,7 +1380,7 @@ impl LockedDatabase {
stream_id: i32,
desired_time: Range<recording::Time>,
forced_split: recording::Duration,
f: &mut dyn FnMut(&ListAggregatedRecordingsRow) -> Result<(), base::Error>,
f: &mut dyn FnMut(ListAggregatedRecordingsRow) -> Result<(), base::Error>,
) -> Result<(), base::Error> {
// Iterate, maintaining a map from a recording_id to the aggregated row for the latest
// batch of recordings from the run starting at that id. Runs can be split into multiple
Expand Down Expand Up @@ -1410,8 +1414,7 @@ impl LockedDatabase {
|| new_dur >= forced_split;
if needs_flush {
// flush then start a new entry.
f(a)?;
*a = ListAggregatedRecordingsRow::from(row);
f(std::mem::replace(a, ListAggregatedRecordingsRow::from(row)))?;
} else {
// append.
if a.time.end != row.start {
Expand Down Expand Up @@ -1450,6 +1453,7 @@ impl LockedDatabase {
}
a.growing = growing;
a.has_trailing_zero = has_trailing_zero;
a.end_reason = row.end_reason;
}
}
Entry::Vacant(e) => {
Expand All @@ -1458,7 +1462,7 @@ impl LockedDatabase {
}
Ok(())
})?;
for a in aggs.values() {
for a in aggs.into_values() {
f(a)?;
}
Ok(())
Expand Down
9 changes: 6 additions & 3 deletions server/db/raw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ const LIST_RECORDINGS_BY_TIME_SQL: &str = r#"
recording.video_samples,
recording.video_sync_samples,
recording.video_sample_entry_id,
recording.open_id
recording.open_id,
recording.end_reason
from
recording
where
Expand All @@ -51,6 +52,7 @@ const LIST_RECORDINGS_BY_ID_SQL: &str = r#"
recording.video_sync_samples,
recording.video_sample_entry_id,
recording.open_id,
recording.end_reason,
recording.prev_media_duration_90k,
recording.prev_runs
from
Expand Down Expand Up @@ -158,11 +160,12 @@ fn list_recordings_inner(
video_sync_samples: row.get(8).err_kind(ErrorKind::Internal)?,
video_sample_entry_id: row.get(9).err_kind(ErrorKind::Internal)?,
open_id: row.get(10).err_kind(ErrorKind::Internal)?,
end_reason: row.get(11).err_kind(ErrorKind::Internal)?,
prev_media_duration_and_runs: match include_prev {
false => None,
true => Some((
recording::Duration(row.get(11).err_kind(ErrorKind::Internal)?),
row.get(12).err_kind(ErrorKind::Internal)?,
recording::Duration(row.get(12).err_kind(ErrorKind::Internal)?),
row.get(13).err_kind(ErrorKind::Internal)?,
)),
},
})?;
Expand Down
3 changes: 3 additions & 0 deletions server/src/json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,9 @@ pub struct Recording {

#[serde(skip_serializing_if = "Not::not")]
pub has_trailing_zero: bool,

#[serde(skip_serializing_if = "Option::is_none")]
pub end_reason: Option<String>,
}

#[derive(Debug, Serialize)]
Expand Down
6 changes: 3 additions & 3 deletions server/src/mp4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ impl FileBuilder {
pub fn append(
&mut self,
db: &db::LockedDatabase,
row: db::ListRecordingsRow,
row: &db::ListRecordingsRow,
rel_media_range_90k: Range<i32>,
start_at_key: bool,
) -> Result<(), Error> {
Expand Down Expand Up @@ -2364,7 +2364,7 @@ mod tests {
"skip_90k={skip_90k} shorten_90k={shorten_90k} r={r:?}"
);
builder
.append(&db, r, skip_90k..d - shorten_90k, true)
.append(&db, &r, skip_90k..d - shorten_90k, true)
.unwrap();
Ok(())
})
Expand Down Expand Up @@ -2492,7 +2492,7 @@ mod tests {
};
duration_so_far += row.media_duration_90k;
builder
.append(&db.db.lock(), row, d_start..d_end, start_at_key)
.append(&db.db.lock(), &row, d_start..d_end, start_at_key)
.unwrap();
}
builder.build(db.db.clone(), db.dirs_by_stream_id.clone())
Expand Down
2 changes: 1 addition & 1 deletion server/src/web/live.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,8 +111,8 @@ impl Service {
let mut rows = 0;
db.list_recordings_by_id(stream_id, live.recording..live.recording + 1, &mut |r| {
rows += 1;
builder.append(&db, &r, live.media_off_90k.clone(), start_at_key)?;
row = Some(r);
builder.append(&db, r, live.media_off_90k.clone(), start_at_key)?;
Ok(())
})?;
}
Expand Down
1 change: 1 addition & 0 deletions server/src/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ impl Service {
video_sample_entry_id: row.video_sample_entry_id,
growing: row.growing,
has_trailing_zero: row.has_trailing_zero,
end_reason: row.end_reason.clone(),
});
if !out
.video_sample_entries
Expand Down
2 changes: 1 addition & 1 deletion server/src/web/view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl Service {
r.wall_duration_90k,
r.media_duration_90k,
);
builder.append(&db, r, mr, true)?;
builder.append(&db, &r, mr, true)?;
} else {
trace!("...skipping recording {} wall dur {}", r.id, wd);
}
Expand Down
19 changes: 17 additions & 2 deletions ui/src/List/VideoList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import TableCell from "@mui/material/TableCell";
import TableRow, { TableRowProps } from "@mui/material/TableRow";
import Skeleton from "@mui/material/Skeleton";
import Alert from "@mui/material/Alert";
import Tooltip from "@mui/material/Tooltip";
import Typography from "@mui/material/Typography";

interface Props {
stream: Stream;
Expand Down Expand Up @@ -40,6 +42,7 @@ export interface CombinedRecording {
height: number;
aspectWidth: number;
aspectHeight: number;
endReason?: string;
}

/**
Expand All @@ -58,7 +61,7 @@ export function combine(
for (const r of response.recordings) {
const vse = response.videoSampleEntries[r.videoSampleEntryId];

// Combine `r` into `cur` if `r` precedes r, shouldn't be split, and
// Combine `r` into `cur` if `r` precedes `cur`, shouldn't be split, and
// has similar resolution. It doesn't have to have exactly the same
// video sample entry; minor changes to encoding can be seamlessly
// combined into one `.mp4` file.
Expand Down Expand Up @@ -100,6 +103,7 @@ export function combine(
height: vse.height,
aspectWidth: vse.aspectWidth,
aspectHeight: vse.aspectHeight,
endReason: r.endReason,
};
}
if (cur !== null) {
Expand Down Expand Up @@ -129,6 +133,7 @@ interface State {
interface RowProps extends TableRowProps {
start: React.ReactNode;
end: React.ReactNode;
endReason?: string;
resolution: React.ReactNode;
fps: React.ReactNode;
storage: React.ReactNode;
Expand All @@ -138,6 +143,7 @@ interface RowProps extends TableRowProps {
const Row = ({
start,
end,
endReason,
resolution,
fps,
storage,
Expand All @@ -146,7 +152,15 @@ const Row = ({
}: RowProps) => (
<TableRow {...rest}>
<TableCell align="right">{start}</TableCell>
<TableCell align="right">{end}</TableCell>
<TableCell align="right">
{endReason !== undefined ? (
<Tooltip title={endReason}>
<Typography>{end}</Typography>
</Tooltip>
) : (
end
)}
</TableCell>
<TableCell align="right" className="opt">
{resolution}
</TableCell>
Expand Down Expand Up @@ -268,6 +282,7 @@ const VideoList = ({
onClick={() => setActiveRecording([stream, r])}
start={formatTime(start)}
end={formatTime(end)}
endReason={r.endReason}
resolution={`${r.width}x${r.height}`}
fps={frameRateFmt.format(r.videoSamples / durationSec)}
storage={`${sizeFmt.format(r.sampleFileBytes / 1048576)} MiB`}
Expand Down
5 changes: 5 additions & 0 deletions ui/src/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,11 @@ export interface Recording {
* the number of bytes of video in this recording.
*/
sampleFileBytes: number;

/**
* the reason this recording ended, if any/known.
*/
endReason?: string;
}

export interface VideoSampleEntry {
Expand Down

0 comments on commit 0422593

Please sign in to comment.