Hi Tristan, On 11/25/24 18:47, Tristan Matthews via ffmpeg-devel wrote:
I haven't done an in-depth review but I tested this locally and it's working well. I'm impressed you were able to implement the packetization without allocating scratch buffers.
I had to rewrite it a couple of times until I got all the cases correct.
One nit I'd add is that since the RTP AV1 spec is still in draft (according to https://aomediacodec.github.io/av1-rtp-spec/) this feature should probably be marked experimental as is done for VP9 in RTP, see: https://git.ffmpeg.org/gitweb/ffmpeg.git/blob/f8e91ab05ff3d111626ab8a3b5d570865a934f07:/libavformat/rtpenc.c#l221 in which case CLI users will have to add `-strict experimental` to their options.
That is a good point and I have added it. Thanks.
For the keyframe detection issue I'm not sure if this is something missing in FFMPEG's RTP stack (e.g. I've noticed that both GStreamer and libwebrtc signal that a buffer contains a keyframe at a higher level), but if not could you set it if you're dealing with a FRAME OBU of type 0 (keyframe) or 2 (intra-only)? You'd need to parse the OBU to extract that however.
I have no problem parsing the few bits in OBU_FRAME and OBU_FRAME_HEADER that are required to detect frame_type == KEY_FRAME instead of looking for OBU_SEQUENCE_HEADER.
Note, that this might be a hit and miss, if (for whatever reasons) the RTP packet size is (unrealistically) small so that the first RTP packet is sent out with other OBUs before the OBU_FRAME[_HEADER] is being handled.
It does not become so unrealistic anymore if there are OBU_METADATAs before the OBU_FRAME[_HEADER] that could contain arbitrary payloads (for such purposes as for signed video). To solve that, all the OBUs in the frame would need to be examined before starting to write the first RTP packet -- which is of course possible. However, it makes questionable how far you would want to go for a workaround instead of looking into the root cause of why the keyframe information flag is currently missing in the RTP state.
I would therefore look into the latter and see if I can pin-point and fix that.
Thanks for your feedback, it is appreciated. I hope to collect more reviews in the upcoming weeks and will come up with an updated patch.
-- Chris _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
