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 761218 - audio/videodecoder: Use gst_pad_peer_query_caps() instead of using gst_pad_get_allowed_caps() to make negotiated output caps before forwarding GAP event
audio/videodecoder: Use gst_pad_peer_query_caps() instead of using gst_pad_ge...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.7.1
Other Linux
: Normal normal
: 1.7.2
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-01-28 04:59 UTC by HoonHee Lee
Modified: 2016-01-28 12:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
use gst_pad_peer_query_caps to make output caps before handling GAP (3.39 KB, patch)
2016-01-28 05:04 UTC, HoonHee Lee
needs-work Details | Review
audio/videodecoder: use gst_pad_peer_query_caps to make output caps (2.66 KB, patch)
2016-01-28 09:13 UTC, HoonHee Lee
needs-work Details | Review
audio/videodecoder: use gst_pad_peer_query_caps to make output caps (2.96 KB, patch)
2016-01-28 10:19 UTC, HoonHee Lee
committed Details | Review
Remove duplicated code (2.89 KB, patch)
2016-01-28 10:38 UTC, HoonHee Lee
committed Details | Review

Description HoonHee Lee 2016-01-28 04:59:37 UTC
Dear All.
When making output caps before handling GAP event, gst_pad_get_allowed_caps() may return NULL if the srcpad has no peer.
It is better to use gst_pad_peer_query_caps() with filter(e.g. source pads template caps) to have negotiated output caps properly before forwarding GAP event. It will never return NULL and we can make output caps properly before forwarding GAP event.
Comment 1 HoonHee Lee 2016-01-28 05:04:40 UTC
Created attachment 319890 [details] [review]
use gst_pad_peer_query_caps to make output caps before handling GAP

Dear Sebastian Dröge and All.
Please check and review this patch.
Thanks.
Comment 2 Sebastian Dröge (slomo) 2016-01-28 08:08:41 UTC
Review of attachment 319890 [details] [review]:

Thanks! :)

::: gst-libs/gst/audio/gstaudiodecoder.c
@@ +2025,3 @@
+  filter = gst_pad_get_pad_template_caps (dec->srcpad);
+  caps = gst_pad_peer_query_caps (dec->srcpad, filter);
+  GST_LOG_OBJECT (dec, "peer caps  %" GST_PTR_FORMAT, caps);

This will still return NULL if there is no peer. My point was that if it returns NULL, you would just use the template caps further here.

Also the checks for empty/any caps is still needed
Comment 3 HoonHee Lee 2016-01-28 09:13:50 UTC
Created attachment 319901 [details] [review]
audio/videodecoder: use gst_pad_peer_query_caps to make  output caps

Dear Sebastian Dröge
Please check and review again.
Thanks.
Comment 4 Sebastian Dröge (slomo) 2016-01-28 09:31:15 UTC
Review of attachment 319901 [details] [review]:

::: gst-libs/gst/audio/gstaudiodecoder.c
@@ +2034,3 @@
+    gst_caps_unref (filter);
+    GST_LOG_OBJECT (dec, "peer caps  %" GST_PTR_FORMAT, caps);
+  }

No, what I mean here is:

templcaps = gst_pad_get_pad_template_caps (srcpad);
caps = gst_pad_peer_query (srcpad, templcaps);
if (caps) {
  gst_caps_unref (templcaps);
  templcaps = NULL;
} else {
  caps = templcaps;
  templcaps = NULL;
}

if (!caps || gst_caps_is_empty (caps) || gst_caps_is_any (caps))
  goto caps_error;


and then all the old code.
Comment 5 HoonHee Lee 2016-01-28 10:19:22 UTC
Created attachment 319909 [details] [review]
audio/videodecoder: use gst_pad_peer_query_caps to make output caps

Dear Sebastian Dröge
Sorry for my misunderstanding. I thought gst_pad_peer_query_caps never return NULL even though there is no peer.

Please review again.
Comment 6 Sebastian Dröge (slomo) 2016-01-28 10:35:56 UTC
Attachment 319909 [details] pushed as 15df3c8 - audio/videodecoder: use gst_pad_peer_query_caps to make output caps
Comment 7 HoonHee Lee 2016-01-28 10:38:24 UTC
Created attachment 319911 [details] [review]
Remove duplicated code

Sorry. 
Please review again.
Comment 8 Sebastian Dröge (slomo) 2016-01-28 12:23:26 UTC
Comment on attachment 319911 [details] [review]
Remove duplicated code

commit dfa2f4952321f72e664946d5537bf0291b24459f
Author: HoonHee Lee <hoonhee.lee@lge.com>
Date:   Thu Jan 28 13:21:33 2016 +0100

    audio/videodecoder: Minor cleanup of last commit
    
    https://bugzilla.gnome.org/show_bug.cgi?id=761218