Skip to content

videodecoders: Add new event that notifies upstream about corrupted streams

Submitted by Owen Williams

Link to original bug (#709711)

Description

Apologies for longish bug report, most of it is a description of a patch that I could write/submit. I'm new here and don't want to start writing a patch that won't be accepted.

+++ What I want:
I want vp8dec and avdec_h264 to send upstream force_key_unit events when they are unable to decode a frame. The force_key_unit event will instruct an upstream element that downstream needs a key frame in order for the decoder to recover (e.g. from packet loss/corrupt frames).

The mechanism to detect corrupt frames is codec specific, e.g. VP8 would use
vpx_codec_control(&dec->decoder, VP8D_GET_FRAME_CORRUPTED, &corrupted).

When decoding problems are detected, each decoder should create a GstEvent using gst_video_event_new_upstream_force_key_unit and push it on the sink pad.

+++ Proposed patch:
So a basic patch would be to add detection code into handle_frame method of the two decoders and push the force_key_unit event on the sink pad.

However I don't want spammy force_key_unit events, I want to be able to configure that a force_key_unit event can happen only once a second. It makes sense to put this logic in the base decoder.

I can provide the following patch:

  1. add min-force-key-unit-interval property to gstvideodecoder.c. This read/write property will get inherited by all videodecoders including vp8dec and avdec_h264.
  2. add gst_video_decoder_corrupt_frame method to gstvideodecoder.c - this should be called by a subclass whenever it cannot decode a frame. It does not replace the need to call gst_video_decoder_finish_frame or gst_video_decoder_drop_frame.
  3. the base class will keep track of the last time a force_key_unit was sent and will only send new force_key_unit if:

if (corrupt_frame->pts >= (last_sent_time + decoder->min_key_unit_interval)) {
//send upstream force_key_unit
}

  1. vp8dec and avdec_h264 are changed to call new gst_video_decoder_corrupt_frame method each time a frame cannot be decoded.

I'm looking for guidance as to whether this patch will likely be acceptable before I start writing it.

One possible objection people may have is that by adding the min-force-key-unit-interval property to the base class, it may look as if other video decoders support sending force_key_unit events, however this won't be the case until the necessary detection logic has been added to the decoder.

Another area of contention is what the default value for min-force-key-unit-interval should be. The safest would probably -1, which means never send force-key-unit events, this would ensure existing pipelines don't start behaving differently.

Whilst I'm currently testing on 1.0.7 the patch would likely be for 1.2

Version: 1.2.0