GNOME Bugzilla – Bug 698679
h264parse doesn't set proper caps on src pad on changes in the sink pad
Last modified: 2013-04-29 14:22:04 UTC
Created attachment 242262 [details] [review] patch that fixes the issue in GStreamer SDK When h264parse is used to convert AVC to bytestream format and the gst_h264_parse_set_caps function is triggered due a caps change (new codec_data and new size) it does the following: 1) new size is extracted and set to h264parse->width and h264parse->height 2) same for PAR and framerate 3) new codec_data is parsed and it's SPS/PPS extracted, the calls to gst_h264_parse_process_nal set h264parse->update_caps to TRUE 4) finally when input format is AVC and converting to bytestream it sets the following: h264parse->push_codec = TRUE; h264parse->have_sps = FALSE; h264parse->have_pps = FALSE; if (h264parse->align == GST_H264_PARSE_ALIGN_NAL) h264parse->split_packetized = TRUE; h264parse->packetized = TRUE; For 0.10 the execution is followed by a call to gst_h264_parse_check_valid_frame which calls to gst_h264_parse_reset_frame and it sets unconditionally h264parse->update_caps to FALSE. For 1.0 the execution is followed by a call to gst_h264_parse_handle_frame which chains to gst_h264_parse_handle_frame_packetized avoiding the call to _reset_frame. At gst_h264_parse_update_src_caps: Case 0: for the first call it sets modified unconditionally Case 1: if update_caps is FALSE early exit (triggered in 0.10 due _reset_frame call) Case 3: there's several checks about the size/fps/par to decide if caps have to be updated. As those checks compare almost the same things size from input caps vs size in the SPS, FPS and PAR modified isn't triggered and caps in the src pad doesn't reflect the change. Although the new SPS/PPS is injected in the pipeline.
The patch attached fixes the issue in 0.10/GStreamer SDK branches. For master only the second change ("if (G_UNLIKELY (modified || h264parse->update_caps)) {" is needed.
So for 1.0 it seems to me like the checks for width/height/FPS/PAR should be extended by a check for changed SPS/PPS overall. Apart from that your patch looks correct too, please push to push master.
It looks like you pushed this: commit b946de72117d423735986407fc2f3c9534bd9939 Author: Josep Torra <n770galaxy@gmail.com> Date: Fri Apr 26 10:38:36 2013 +0200 h264parse: Update src pad caps when it was explicitly signaled Fixes src pad caps aren't updated when converting from AVC to bytestream and new caps had been received in the sink pad. https://bugzilla.gnome.org/show_bug.cgi?id=698679 What about Sebastian's comments? Is there anything else to do or can this be close? (Did you test it on 1.x?)
Sorry I've got distracted on something else and I've forgot to handle the bug report properly. Well I'm not sure how to implement the SPS/PPS change detection with the current code structure although the minimal change I've introduced should handle it properly. Unfortunately I can't test the change in 1.0 because I don't have ported yet the elements that triggered the scenario in 0.10. But I think that the fix is correct and shouldn't cause undesired side effects. I'm closing the bug now.
commit b946de72117d423735986407fc2f3c9534bd9939 Author: Josep Torra <n770galaxy@gmail.com> Date: Fri Apr 26 10:38:36 2013 +0200 h264parse: Update src pad caps when it was explicitly signaled Fixes src pad caps aren't updated when converting from AVC to bytestream and new caps had been received in the sink pad. https://bugzilla.gnome.org/show_bug.cgi?id=698679