Allow to get/set average frame rate

[rkfg]

This is important for containers with predefined timebase like Matroska with fixed 1 ms timebase. Without setting the average FPS manually you'll get 1000 fps (or 1k) as a result. It's usually not the true FPS but rather a best guess by the tools like ffprobe. Still, it looks ugly and can confuse less intelligent tools like TV software. Note that setFrameRate() doesn't help in this case hence introducing a new method.

[h4tr3d]

Hi @rkfg! Thanks for patch. I merge it now, but... could you please provide some sample for this API using? I suggest take a some exists one, rename and show this API using. Thanks!

[rkfg]

Oh, sure! This was my personal pain and took me a whole day to figure out while digging through the ffmpeg (both libav and the cli utility) sources but the damn thing always put 1k fps no matter what I set. And the server at the local TV station was going mad because of 1000 fps. I found that it's a hardcoded mkv timebase and there's no way to change it without changing the source. Still, all other mkvs I had on hand sure showed normal fps like 25 or 30. So I was stuck for days but in the end left it as is because it worked good enough for us.

At the time I just put 1/25 timebase to the stream parameters after writing the headers and while it worked it was incorrect and hacky. Like, mkvinfo said I had 0.9 sec of video and 35 sec of audio (the true duration) and mpv showed a lot of dropped frames. But finally I found the line in the ffmpeg utility that set this parameter and to my surprise it helped! That's the story. I'll provide an example, though it mostly applies to mkv only (which we need because of subtitles), MP4 afaik doesn't have a forced timebase.

[rkfg]

Hmm, now that I started to look into it more closely it becomes all confusing. This source says that r_frame_rate is actually deprecated and we should use avg_frame_rate instead. What do you think? I guess it would be much more convenient to just replace (or add) the former with the latter inside Stream::setFrameRate() and call it a day. Take a look at this: they set both fields to the same value err, not the same but they copy them both. But actually setting r_frame_rate only does not affect the FPS, at least as the tools like ffprobe/mpv report. Other usages of avg_frame_rate are this and this.

Try encoding an MKV file with this patch:

diff --git a/example/api2-samples/api2-scale-video.cpp b/example/api2-samples/api2-scale-video.cpp
index 0dda886..bf48f30 100644
--- a/example/api2-samples/api2-scale-video.cpp
+++ b/example/api2-samples/api2-scale-video.cpp
@@ -93,7 +93,7 @@ int main(int argc, char **argv)
     OutputFormat  ofrmt;
     FormatContext octx;
 
-    ofrmt.setFormat("flv", out);
+    ofrmt.setFormat("mkv", out);
     octx.setFormat(ofrmt);
 
     Codec        ocodec  = findEncodingCodec(ofrmt);
@@ -129,7 +129,7 @@ int main(int argc, char **argv)
     //
     // RESCALER
     //
-    VideoRescaler rescaler; // Rescaler will be inited on demaind
+    VideoRescaler rescaler(encoder.width(), encoder.height(), encoder.pixelFormat());
 
 
     //

I fixed the rescaler, the on demand init doesn't actually work as the rescaler doesn't know the target width/height (and I couldn't figure out why it should in the first place). So, I guess replacing or adding the avg_frame_rate field in the getter/setter is really a way to go and it behaves just the way the user expects it.

[rkfg]

Oh, I really don't know; another check of the encoded file (without setting the avg_frame_rate manually it's probably derived directly from the timebase of the stream): ffprobe -v error -select_streams v:0 -show_entries stream=r_frame_rate,avg_frame_rate -of default=nw=1 test.mkv

r_frame_rate=30/1
avg_frame_rate=1000/1

So both fields are there and they must have different meanings and use cases. That's fine but I think about making it easier to use for the user because this discrepancy is really obscure and is not documented well. Maybe it's still fine to have two getters and setters but we need to spice it up with some commentary about what is what.

Also: this SO question and this, linked from there. It seems to be tied to the encoder used. Well, shit.

[h4tr3d]

Just collect useful links here.

• https://www.bountysource.com/issues/3200677-ffdecoder-use-avg_frame_rate-instead-of-r_frame_rate
• https://gitlab.com/ember-dev/kodi/commit/491745da95d93cd95a1d85b5db4d9995e749e822
• http://www.ffmpeg-archive.org/Problem-retrieving-correct-fps-while-streaming-cameras-td4675716.html - OpenCV uses r_frame_rate for old FFmpeg versions and avg_frame_rate for new one.

[rkfg]

Regarding OpenCV, that's for getting the frame rate but it doesn't say anything about setting one which is the point of this PR.

This line from kodi is interesting:

if (m_bMatroska && pStream->avg_frame_rate.den && pStream->avg_frame_rate.num)

So this parameter is only used (or important) explicitly for MKV it seems. But I couldn't find a way to access the format context from within the stream, and I'm not sure it's a good idea either in terms of incapsulation. We could add a FormatContext reference to the Stream class so it could check what the outer format is and apply the framerate accordingly but it smells of overengineering to me. As a compromise, we could add setFramerate() to FormatContext directly (which just sets the required field according to the mkv/non-mkv format difference) as the vast majority of videos only contain one videostream and thus need only one frame rate. Then again, that violates incapsulation and is not good for getting information if we have more than one videostream.

About the original request concerning an API example, would it be ok to just modify the existing one so that it rescales video and saves it to an MKV file (instead of FLV)? I made just that and also added comments about setting the framerate and why it's important.