GNOME Bugzilla – Bug 765365
h264parse: Fails to parse HRD parameters in SPS that ffmpeg accepts just fine
Last modified: 2018-11-03 13:49:40 UTC
Please try this pipeline using the provided public url gst-launch-1.0 -vmt rtspsrc location="rtspt://admin:q1w2e3r4t5y6@93.63.189.11:45433/onvif/profile5/media.smp" ! rtph264depay ! h264parse ! matroskamux ! fakesink silent=false you'll get a not negotiated error from rtspsrc if you remove matroskamux h264parse probably works in passthrough mode and the stream can be played
Problem is (at least) that h264parse does not put width/height into the caps, matroskamux needs them.
Reasons is that parsing the SPS fails. The VUI has invalid HRD. hrd->cpb_cnt_minus1 is 1, so 2 are parsed. But the second fails right in the beginning in gst_h264_parse_hrd_parameters(). Changing that loop to < instead of <= makes it work.
Created attachment 326474 [details] [review] annotations
Sree, can you take a look at those two parts of the code and compare it to the spec?
Comment on attachment 326474 [details] [review] annotations > >+ /* FIXME Works if making this < instead of <= */ > for (sched_sel_idx = 0; sched_sel_idx <= hrd->cpb_cnt_minus1; sched_sel_idx++) { No, the existing code is right: as per spec for( SchedSelIdx = 0; SchedSelIdx <= cpb_cnt_minus1; SchedSelIdx++ ) {
Review of attachment 326474 [details] [review]: ::: gst-libs/gst/codecparsers/gsth264parser.c @@ +1950,3 @@ gint i; + /* FIXME Shouldn't this be <= ? */ As per Spec, if( slice_group_map_type = = 2 ) for( iGroup = 0; iGroup < num_slice_groups_minus1; iGroup++ )
Ok, so it's all behaving according to the spec. Maybe we should ignore parsing errors in that code then?
(In reply to Sebastian Dröge (slomo) from comment #7) > Ok, so it's all behaving according to the spec. Maybe we should ignore > parsing errors in that code then? Hm, we have more issues, vaapidecode will fail since it is also parsing headers..
As ffmpeg can handle the stream just fine, it should somehow be possible to ignore the problems :)
the macro READ_UE and READ_UINT8 have a goto error if parsing fails, maybe we can create a new macro without the goto or use directly nal_reader_get_ue/nal_reader_get_bits_uint8 inside the for, ignoring the errors make the parser works properly, something like this (note the additional read): for (sched_sel_idx = 0; sched_sel_idx <= hrd->cpb_cnt_minus1; sched_sel_idx++) { //READ_UE (nr, hrd->bit_rate_value_minus1[sched_sel_idx]); nal_reader_get_ue (nr, &hrd->bit_rate_value_minus1[sched_sel_idx]); //READ_UE (nr, hrd->cpb_size_value_minus1[sched_sel_idx]); nal_reader_get_ue (nr, &hrd->cpb_size_value_minus1[sched_sel_idx]); READ_UINT8 (nr, hrd->cbr_flag[sched_sel_idx], 1); nal_reader_get_bits_uint8 (nr, &hrd->cbr_flag[sched_sel_idx], 1); } are these fields optional? For your info I reported the issue to the camera manufacter too
They are "optional" as in there is a flag elsewhere that specifies if they are there or not. The flag is set and then they must be there.
HRD parameters are the part of sps headers. We have to handle it carefully. May be a field in sps to indicate the vui parsing isn't success and also generate the warning when it failed to parse VUI. Because we have to make sure sps parsing returns TRUE , but never use any vui related parameters since it wrongly encoded. Or an ugly hack by setting vui_parameters_present_flag = FALSE if any failure in vui parsing.
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/issues/379.