scottlamb / moonfire-nvr

Moonfire NVR, a security camera network video recorder
Other
1.26k stars 138 forks source link

Unable to test or record the stream due to negative rtptime value #224

Closed MarsRover75 closed 2 years ago

MarsRover75 commented 2 years ago

Describe the bug Unable to test or start the stream (sometimes). Error message:

main stream at rtsp://10.98.66.32:554/1/1:
[10.98.66.15:37222(me)->10.98.66.32:554@2022-05-07T02:23:04, 1347@2022-05-07T02:23:04] Ok response to PLAY CSeq=4: bad rtptime "-426852246"

To Reproduce Unfortunately there's no way to reproduce it without the camera (OMNY M5S2A 2812). For some reason this type of cameras sometimes (actually >50% of times) return negative value for rtptime.

Expected behavior Handle negative values for rtptime some other way than it's done now. In retina client/parse.rs lines 645-646:

                    let rtptime = u32::from_str_radix(value, 10)
                        .map_err(|_| format!("bad rtptime {:?}", value))?;

Other RTSP recording/capturing/viewing software works just fine in this situation (e.g. ZoneMinder, VLC Player)

Server:

# docker ps
CONTAINER ID   IMAGE                           COMMAND                  CREATED        STATUS         PORTS     NAMES
b80bf5bcb3d9   scottlamb/moonfire-nvr:v0.7.4   "/usr/local/bin/moon…"   23 hours ago   Up 4 seconds             moonfire-nvr
# docker images
REPOSITORY               TAG       IMAGE ID       CREATED        SIZE
scottlamb/moonfire-nvr   v0.7.4    e4622e7df76a   3 weeks ago    360MB

Camera: OMNY M5S2A 2812

Desktop

scottlamb commented 2 years ago

Interesting. RTP RFC 3550 defines time as an unsigned value, e.g. the appendix says:

       u_int32 ts;               /* timestamp */

and RTSP RFC 2326 says the rtptime parameter can't have a minus:

   parameter       = ";" "seq" "=" 1*DIGIT
                   | ";" "rtptime" "=" 1*DIGIT

Looks like ffmpeg just ignores the error and sets rtptime to 0:

        else if (!strcmp(key, "rtptime"))
            rtptime = strtoul(value, NULL, 10);

We could similarly pretend rtptime isn't there. Or, I guess, when there's a -, parse it as an i32 then cast it (essentially adding 232 to the value).

MarsRover75 commented 2 years ago

I'm aware that RFC defines it as uint32. No idea why this type of cameras set it to negative, the purpose of doing this is totally beyond my control and my understanding, but I think the safest and sanest way to handle this behavior would be to try parse it as signed int (or even float) and then set it to 0 if it violates the RFC in any way: negative, fractional (maybe round it if it's positive), unparsable, you name it.

scottlamb commented 2 years ago

I believe 0.7.5 will address this by ignoring the bad value. Please try it and let me know—it's certainly possible that there's something else wrong when talking with this camera.

MarsRover75 commented 2 years ago

Thanks for that Scott! I've rebooted those cameras and so far they behave normally, I guess I'll have to wait for this bug to reappear.

scottlamb commented 2 years ago

Sounds good. H.264 streams use 90 kHz clocks, so when stored as a 32-bit signed integer, they should flip between positive and negative every ~6.6 hours.

MarsRover75 commented 2 years ago

wow, thanks for clarifying!