Opened 11 years ago

Closed 10 years ago

#6243 closed patch (fixed)

H.264 recording fixes

Reported by: jppoet@… Owned by: Janne Grunau
Priority: minor Milestone: 0.22
Component: MythTV - General Version: unknown
Severity: medium Keywords: H.264
Cc: Ticket locked: no

Description

Various fixes and clean up of H264Parser.

The detection of the start of a AU is now more correct, even though functionally the previous code produced the same results.

The first IDR will now be returned, instead of waiting for the next I-frame.

Technically, I-frames in H.264 are not keyframes. The HD-PVR only has an IDR every 128 frames, and does not seem to have any "recovery point" SEI NALs. That is a long time between keyframes, so I have left in the use of I-frames as keyframes. It is a #ifdef so can be removed easily.

Added parsing of "recovery point" SEI NALs, which should allow more accurate keyframe detection for BBC HD, and other streams without IDRs --- but I cannot test this. If someone in the UK could test this patch, and check their seektable to see if reasonable entries are being made there, I would appreciate it. This part is loosely based on recent patches to ffmpeg.

Attachments (2)

H264Parser-fixes.patch (11.5 KB) - added by jppoet@… 11 years ago.
H264Parser improvements
H264Parser-fixes-v1.1.patch (12.2 KB) - added by jppoet@… 11 years ago.
Allows recorder to designate if I frames should be counted as keyframes

Download all attachments as: .zip

Change History (9)

Changed 11 years ago by jppoet@…

Attachment: H264Parser-fixes.patch added

H264Parser improvements

comment:1 Changed 11 years ago by mortalmatt

I've been running mythtv with this patch for a day or so now without any hitch. BBC HD runs as good or as bad as before- AFAICT now. Is there anything in particular you want to get tested or anything one should pay closer attention to...?

comment:2 Changed 11 years ago by jppoet@…

Thank you, mortalmatt, for testing it.

Can you try commenting out (or removing) the

#define USE_I_FRAME_AS_KEYFRAME 1

at the top of H264Paser.cpp, and see if it still works?

If it still works, I would be curious to know how far apart it thinks your BBC HD keyframes are. You could tell by looking at the seektable entries for a BBC HD show, or by EDITING a show you are watching, and telling it to move forward/backward one keyframe at a time.

comment:3 Changed 11 years ago by Matthias "mortalmatt" Dahl <devel@…>

You're welcome. I just made a few tests and it seems like everything is, again, as good or as bad as before. I made a few test recordings and looked at the seektable entries. Now I have no idea if this is right but all recordings (either SD or (BBC) HD) have type 9 (GOPBYFRAME) and there are no KEYFRAME types there.

Nevertheless... it detects GOPBYFRAME exactly 12 frames (mark) apart which it also did without your patch.

Please, if I made a mistake somewhere, let me know and I'll happily try again.

comment:4 Changed 11 years ago by Matthias "mortalmatt" Dahl <devel@…>

Ok... tried the editing. If I choose keyframe there, it assumes a keyframe every 24 frames with and without USE_I_FRAME_AS_KEYFRAME set.

Changed 11 years ago by jppoet@…

Attachment: H264Parser-fixes-v1.1.patch added

Allows recorder to designate if I frames should be counted as keyframes

comment:5 Changed 11 years ago by jppoet@…

There has been a fare amount of discussion on the ffmpeg list lately about what constitutes a keyframe in H.264. While technically, I frames are not keyframes, they seem to have concluded that some H.264 streams do not have proper keyframes, so there is not much choice but to use I frames.

One suggestion was to start using I frames if neither an IDR or a SEI recovery point is seen for X number of seconds. It takes the HD-PVR over two seconds to produce an IDR (every 128 frames for 720p). While that is probably (technically) acceptable, I find that seeking in HD-PVR recordings is a little smoother if I frames are used as keyframes.

I have updated the patch on the ticket to allow different recorders to designate if I frames should be used as keyframes. By default they are not, but the HD-PVR recorder will enable that flag.

mortalmatt, thank you for your testing. In theory, nothing has changed in this new version for BBC HD recordings.

comment:6 Changed 10 years ago by Janne Grunau

Milestone: unknown0.22
Owner: changed from Isaac Richards to Janne Grunau
Status: newaccepted

comment:7 Changed 10 years ago by Janne Grunau

Resolution: fixed
Status: acceptedclosed

(In [21011]) Adds SEI parsing to H264Parser to mark SEI recovery points as Keyframes. Fixes #6243

adds an option to disable using I slices as keyframes and other various cleanups patch by: John P Poet < jppoet [] gmail >DOT< com >

Note: See TracTickets for help on using tickets.