This would seem like a bug in discoverer then. The decodebin change only makes it so that the topology actually contains the pads it's supposed to contain instead of NULL.

(In reply to Sebastian Dröge (slomo) from comment #1)
> This would seem like a bug in discoverer then. The decodebin change only
> makes it so that the topology actually contains the pads it's supposed to
> contain instead of NULL.

Well this is the theory to me :)

# Old topology:

stream-topology, next=(structure){
  "stream-topology, next=(structure)"stream-topology, caps=(GstCaps)subpicture/x-pgs, pad=(GstPad)"(GstDecodePad) src_2",
    element-srcpad=(GstPad)"(GstPad) src_2";", caps=(GstCaps)subpicture/x-pgs, element-srcpad=(GstPad)"(GstPad) subpicture_0_1200";",
  "stream-topology, next=(structure)"stream-topology, caps=(GstCaps)"audio/x-raw, format=(string)F32LE, layout=(string)interleaved, rate=(int)[ 4000, 96000 ], channels=(int)[ 1, 6 ]", pad=(GstPad)"(GstDecodePad#
    element-srcpad=(GstPad)"(GstPad) src";", caps=(GstCaps)audio/x-ac3,
    element-srcpad=(GstPad)"(GstPad) audio_0_1100";",
  "stream-topology, next=(structure)"stream-topology, caps=(GstCaps)"video/x-raw, format=(string){ I420, YUY2, RGB, BGR, Y42B, Y444, YUV9, Y41B, GRAY8, RGB8P, I420, Y42B, Y444, UYVY, NV12, NV21, ARGB, RGBA, ABG#
    element-srcpad=(GstPad)"(GstPad) src";", caps=(GstCaps)"video/x-h264, stream-format=(string)byte-stream, alignment=(string)nal",                                                                               
    element-srcpad=(GstPad)"(GstPad) video_0_1011";"
  }, caps=(GstCaps)"video/mpegts, systemstream=(boolean)true, packetsize=(int)192", element-srcpad=(GstPad)"(GstPad) src";


# Topology since 46835f550d0a49338a3477973df842b1443c7a81

0:00:02.553599408 31385 0x55edcfa4db00 ERROR             discoverer gstdiscoverer.c:1320:discoverer_collect: Topology is stream-topology,
    next=(structure){
      "stream-topology, next=(structure)"stream-topology, caps=(GstCaps)subpicture/x-pgs, pad=(GstPad)"(GstDecodePad) src_2",
        element-srcpad=(GstPad)"(GstPad) src_2";", caps=(GstCaps)subpicture/x-pgs,
        element-srcpad=(GstPad)"(GstPad) subpicture_0_1200";",
      "stream-topology, next=(structure)"stream-topology, next=(structure)"stream-topology, caps=(GstCaps)"audio/x-raw, format=(string)F32LE, layout=(string)interleaved, rate=(int)[ 4000, 96000 ], channels=(int#
        element-srcpad=(GstPad)"(GstPad) src";", caps=(GstCaps)"audio/x-ac3, framed=(boolean)true, rate=(int)48000, channels=(int)6, alignment=(string)frame";",
        element-srcpad=(GstPad)"(GstPad) audio_0_1100", caps=(GstCaps)audio/x-ac3;",
      "stream-topology, next=(structure)"stream-topology, next=(structure)"stream-topology, caps=(GstCaps)"video/x-raw, format=(string){ I420, YUY2, RGB, BGR, Y42B, Y444, YUV9, Y41B, GRAY8, RGB8P, I420, Y42B, Y#
        element-srcpad=(GstPad)"(GstPad) src";", caps=(GstCaps)"video/x-h264, stream-format=(string)avc, alignment=(string)au, pixel-aspect-ratio=(fraction)1/1, width=(int)1920, height=(int)1080, framerate=(fra#
        element-srcpad=(GstPad)"(GstPad) video_0_1011", caps=(GstCaps)"video/x-h264, stream-format=(string)byte-stream, alignment=(string)nal";"
    },
    caps=(GstCaps)"video/mpegts, systemstream=(boolean)true, packetsize=(int)192", element-srcpad=(GstPad)"(GstPad) src";

So here we can see that we used not to have caps for the repetition of the srcpad stream and we now have. The discoverer is not able to handle that change because h264parse is doing some conversion, this 'hack' would solve our issue here though:

> diff --git a/gst-libs/gst/pbutils/gstdiscoverer.c b/gst-libs/gst/pbutils/gstdiscoverer.c
> index 96fa39712..bf55a0fa8 100644
> --- a/gst-libs/gst/pbutils/gstdiscoverer.c
> +++ b/gst-libs/gst/pbutils/gstdiscoverer.c
> @@ -1070,6 +1070,8 @@ child_is_same_stream (const GstCaps * _parent, const GstCaps * child)
>    for (i = 0; i < size; i++) {
>      gst_structure_remove_field (gst_caps_get_structure (parent, i), "parsed");
>      gst_structure_remove_field (gst_caps_get_structure (parent, i), "framed");
> +    gst_structure_remove_field (gst_caps_get_structure (parent, i), "stream-format");
> +    gst_structure_remove_field (gst_caps_get_structure (parent, i), "alignment");
>    }
>    res = gst_caps_can_intersect (parent, child);
>   gst_caps_unref (parent);

It might make sense but I am not sure, what do you think?

Yes that makes sense. There's probably other things that should also be removed though.
I remember doing similar things in encodebin, maybe also check there?


Do you still get different caps and two streams here if the input has e.g. no profile but h264parse adds one?

> Yes that makes sense. There's probably other things that should also be removed though.
> I remember doing similar things in encodebin, maybe also check there?

I checked and the only thing that seems to be filter there is the streamheader, I do not think it should be filter in our case as we should have the same one (or none which is also fine) before and after the parser.

> Do you still get different caps and two streams here if the input has e.g. no profile but h264parse adds one?

I do not have this case handy, any idea where I could find that?

I guess I would but then it is not a problem if fields are added as intersection would work afaiu.

Created attachment 345231 [details] [review]
discoverer: Ignore more parser related fields when comparing streams

The parser might do some conversion on a stream but the stream keeps
being the same, and we need to make sure GstDiscoverer detects it is the
case.

Attachment 345231 [details] pushed as d5fbc90 - discoverer: Ignore more parser related fields when comparing streams