After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 773666 - baseparse: fix draining with less data than min frame size available
baseparse: fix draining with less data than min frame size available
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gstreamer (core)
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-10-29 10:25 UTC by Tim-Philipp Müller
Modified: 2018-11-03 12:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
baseparse: fix draining with less data than min frame size available (2.06 KB, patch)
2016-10-29 10:25 UTC, Tim-Philipp Müller
committed Details | Review
tests: rawvideoparse: add test for flow error handling (2.26 KB, patch)
2016-10-29 10:35 UTC, Tim-Philipp Müller
committed Details | Review

Description Tim-Philipp Müller 2016-10-29 10:25:04 UTC
Created attachment 338771 [details] [review]
baseparse: fix draining with less data than min frame size available

Came across this when debugging some issues with rawvideoparse in a not-negotiated flow error scenario:

gst-launch-1.0  fakesrc sizetype=random ! queue ! rawvideoparse format=rgb ! appsink caps=video/x-raw,format=I420

First it would not return the flow error properly. After fixing that it would hit an assert in rawvideoparse

gstrawbaseparse.c:519:gst_raw_base_parse_handle_frame: assertion failed: (in_size >= frame_size)

Reason is that basesrc sends an EOS in response to the flow error, which makes basesrc drain, and the draining code passes less than the configured minimum framesize to the rawvideoparse subclass, which is unexpected.

Not tested the patch a lot, so saving it for now.
Comment 1 Tim-Philipp Müller 2016-10-29 10:35:15 UTC
Created attachment 338772 [details] [review]
tests: rawvideoparse: add test for flow error handling
Comment 2 Sebastian Dröge (slomo) 2016-11-01 18:34:15 UTC
Comment on attachment 338771 [details] [review]
baseparse: fix draining with less data than min frame size available

Attachment 338771 [details] pushed as 2e278ae - baseparse: fix draining with less data than min frame size available
Comment 3 Sebastian Dröge (slomo) 2016-11-01 18:34:50 UTC
Attachment 338772 [details] pushed as 280b4ac - tests: rawvideoparse: add test for flow error handling
Comment 4 Sebastian Dröge (slomo) 2016-11-01 21:41:36 UTC
This breaks the aacparse and amrparse unit tests
Comment 5 Tim-Philipp Müller 2016-11-01 21:53:10 UTC
Will have a look. Told you I didn't test it beyond rawparse ;)
Comment 6 Sebastian Dröge (slomo) 2016-11-02 07:34:57 UTC
I think this has to be come a setting. Various audio parsers assume currently to get any data left at EOS and pass them onwards. Reverting for now and then we can add a property :)
Comment 7 Sebastian Dröge (slomo) 2016-12-16 16:13:50 UTC
So have to add a setting here and use it in all the parsers where it makes sense. However I'm not sure if it makes sense how aacparse for example is using it. It will output incomplete AAC frames.
Comment 8 Sebastian Dröge (slomo) 2017-02-18 18:20:12 UTC
commit 63e280df22073f06dc35a5cab03dad5d509eca71
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Sat Feb 18 20:18:50 2017 +0200

    rawbaseparse: Drop incomplete frames at EOS
    
    See https://bugzilla.gnome.org/show_bug.cgi?id=773666
    
    This would ideally be solved in baseparse but that requires further
    thought at this point, and in the meantime it would be good to have
    rawbaseparse not assert on this but handle it gracefully instead.
Comment 9 GStreamer system administrator 2018-11-03 12:37:54 UTC
-- 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/gstreamer/issues/202.