Closed scottlamb closed 1 month ago
If you pull latest Retina, the mp4 example should no longer produce this error.
Progress. I updated Moonfire's recording logic. Wasn't as bad as I feared. It no longer disconnects and reconnects when the parameters change, instead creating a new recording within the run. That's assuming the parameters change at a key frame. If they change somewhere else...well, then it'll be a bit more involved.
Currently the UI's list view will display a new row in the table any time the parameters change, and it won't let you create a single .mp4
file that spans the change (even though the /view.mp4
endpoint in the API should handle that fine). This is basically because it displays the pixel dimensions for each row. We could alter it to only split up the run into multiple rows when the dimensions actually change. A future scrub bar UI (#32) also should handle this seamlessly.
The live video will break on parameter change right now. Here's the offending code:
The recording change is in the freshly released v0.7.4.
In https://github.com/scottlamb/moonfire-nvr/issues/213#issuecomment-1094110481, @IronOxidizer wrote:
Out of curiosity, I parsed the
extra_data
above to see what changed:The H.264 spec says:
What QPY actually does is a little beyond my understanding, so let's just say it changes the image somehow.
Basically the camera is (re)setting a default parameter that may be used or always overridden is used in each macroblock (16x16 pixel block of an image). Of course, altering our code to inspect all the macroblocks to know which before deciding how to handle the parameter change would be madness. The only reasonable assumption is that the parameter change is important.[edit: actually, even if QPY is later adjusted, the new value is written as a delta from the current. So the parameter change really does affect all following frames no matter what.]
Cameras are allowed to change video parameters mid-stream, although this is the first time I've seen one decide to on its own. Some of my cameras will do a mid-stream parameter change only if you actually change the image settings in the camera's config UI. The rest won't even then—they'll drop all active RTSP streams instead, forcing a reconnect. Moonfire's current approach is to reconnect on a mid-stream parameter change, basically treating the former like the latter. It's been good enough so far. But sounds like this camera might have extremely frequent drops if we don't rethink this approach.
The right way to handle this is to, at the
.mp4
layer, plumb through the new parameters into a fresh "sample description" (terminology from ISO 14496-12). The Retina library is fine—it's already passing along that the parameters changed. The Retinamp4
example needs to change here. Moonfire's recording logic needs to change here to seamlessly change parameters without reconnecting, which will require some thought to plumb through properly to the layer above. And on playback, the.mp4
generation logic might already handle this fine but I haven't really tested this case. Likewise the live stream logic in the UI layer.I think most players would let us get away without doing it the right way. Moonfire currently isn't stripping the video parameters out of the actual video frames (see #43). I think a lot of video players basically concatenate the sample descriptions together with the video frames, so basically the H.264 decoder won't even notice the wrong sample descriptions because they're immediately overridden. In other words, if we commented out that
bail!
line in Moonfire code so it just didn't pay attention to this parameter change (that notably doesn't affect anything like image resolution that Moonfire pays particular attention to), this would work fine for your camera. Probably.I'd like to handle this the right way. Not sure yet how quickly it can happen. I've got some other things going on like finishing my taxes next week.