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 707229 - flacparse: turns all GstFlowResult other than OK to error
flacparse: turns all GstFlowResult other than OK to error
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.0.11
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-09-01 15:40 UTC by Matej Knopp
Modified: 2013-09-03 16:07 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
Patch (815 bytes, patch)
2013-09-01 15:40 UTC, Matej Knopp 		rejected Details | Review
Patch (3.41 KB, patch)
2013-09-02 22:19 UTC, Matej Knopp 		none Details | Review
Patch (3.41 KB, patch)
2013-09-03 01:36 UTC, Matej Knopp 		committed Details | Review
Patch to move cleanup on error after state change (2.02 KB, patch)
2013-09-03 15:34 UTC, Matej Knopp 		committed Details | Review

Description Matej Knopp 2013-09-01 15:40:38 UTC 
Created attachment 253759 [details] [review]
Patch

gst_flac_parse_handle_headers returns false when flow is anything but OK when pushing headers
so even for not linked, etc it returns false
and when it does, gst_flac_parse_parse_frame doesn't change res from GST_FLOW_ERROR
Comment 1 Sebastian Dröge (slomo) 2013-09-02 09:58:08 UTC 
commit 1971c43279db2f3d53eec47a031618a742a43565
Author: Sebastian Dröge <slomo@circular-chaos.org>
Date:   Mon Sep 2 11:46:52 2013 +0200

    flacparse: Properly propagate downstream flow returns upstream
    
    https://bugzilla.gnome.org/show_bug.cgi?id=707229
Comment 2 Sebastian Dröge (slomo) 2013-09-02 09:58:24 UTC 
Comment on attachment 253759 [details] [review]
Patch

not exactly, you should return exactly what downstream returned to upstream :)
Comment 3 Matej Knopp 2013-09-02 22:19:09 UTC 
Created attachment 253907 [details] [review]
Patch

Is this any better?
Comment 4 Matej Knopp 2013-09-03 01:36:28 UTC 
Created attachment 253913 [details] [review]
Patch
Comment 5 Sebastian Dröge (slomo) 2013-09-03 08:13:39 UTC 
This is basically what I pushed :) Your patch additionally fixes a memleak though:

commit 73751dbbe7d37d7847e22e81e3faf793af48bbb8
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Tue Sep 3 10:10:01 2013 +0200

    flacparse: Free GstBaseParseFrame if pushing a header failed
Comment 6 Matej Knopp 2013-09-03 15:34:39 UTC 
Created attachment 253985 [details] [review]
Patch to move cleanup on error after state change

There is slight problem with the committed patch, when sending buffer fails for some reason the parser state never changes. So when I run it with non linked pad I get log full of these:

missing header 0x0 0x0 0x4caa510, muxing into container formats may be broken
Comment 7 Sebastian Dröge (slomo) 2013-09-03 16:07:12 UTC 
commit 349afc633a8d735eecf7e1b4744e6e82bf6f4a81
Author: Matej Knopp <matej.knopp@gmail.com>
Date:   Tue Sep 3 17:32:41 2013 +0200

    flacparse: cleanup on error after state change
    
    https://bugzilla.gnome.org/show_bug.cgi?id=707229