GNOME Bugzilla – Bug 779831
h264parse (baseparse?) fail to pass not-linked upstream sometimes
Last modified: 2017-03-24 09:57:48 UTC
gst-launch-1.0 filesrc location=test-output.ts ! tsdemux ! h264parse ! identity -v This pipeline should error out with not-linked, but it doesn't. The problem seems to be specific to processing byte-stream data with alignment=nal. With either avc or alignment=au, it will return not-linked as expected.
I found a sample which reproduces that, and the bug comes from filler data being marked for dropping by h264parse then dropped by baseparse, which returns GST_FLOW_OK since it doesn't actually push anything, thereby overriding the flow result ultimately returned. It's not quite clear how that should be fixed. The obvious way would be to keep a "last_ret" field in baseparse, and return this when dropping or otherwise not pushing, instead of OK.
Created attachment 348155 [details] [review] do not ignore flow errors when dropping data
Review of attachment 348155 [details] [review]: ::: gst/videoparsers/gsth264parse.c @@ +1032,3 @@ + /* Dropping will return GST_FLOW_OK */ + gst_base_parse_finish_frame (parse, frame, map.size); + ret = h264parse->last_ret; Why do we even get here if non-OK was returned previously? After ERROR was returned, it should have returned that to upstream already.
Created attachment 348166 [details] [review] baseparse: Don't forget error returns when processing more If parsing returns a non-OK flow return in the middle of processing an input buffer, don't overwrite that if a later return is OK again - the subclass might return not-linked in the middle, and then discard subsequent data without pushing while returning OK. A later success doesn't invalidate the earlier failure, but we should continue processing after not-linked, so as to keep parse state consistent.
(In reply to Sebastian Dröge (slomo) from comment #3) > Review of attachment 348155 [details] [review] [review]: > > ::: gst/videoparsers/gsth264parse.c > @@ +1032,3 @@ > + /* Dropping will return GST_FLOW_OK */ > + gst_base_parse_finish_frame (parse, frame, map.size); > + ret = h264parse->last_ret; > > Why do we even get here if non-OK was returned previously? After ERROR was > returned, it should have returned that to upstream already. The problem is baseparse keeps processing more of the input packet on not-linked, and eventually returns whatever the last packet returns
Comment on attachment 348166 [details] [review] baseparse: Don't forget error returns when processing more That makes more sense to me