Created attachment 166142 [details] [review]
proposed patch for adding 'caps' property for playbin

proposed patch for adding 'caps' property for playbin

I don't think this is the right approach. Instead, if anything should be exposed, the autoplug-* signals of uridecodebin/decodebin2 should be exposed to the application.

sorry, Dröge. I don't catch your idea clearly.
what's detail of your idea to let decodebin2 expose?

Well, after another thought nothing of uridecodebin/decodebin2's autoplug-* signals or the caps should be exposed.

As Edward has answered to your mail to gst-devel (and see my latest comment to your other bug), this should all be handled automagically already if your sink is set as video sink to playbin2 and behaves as playbin2 expects.

There are two ways to run into expose_pad in gstdecodebin2.c
1. analyze_new_pad()
  /* 1.a if autoplug-continue is FALSE or caps is a raw format, goto pad_is_final */
  if ((!apcontinue) || are_raw_caps (dbin, caps))
    goto expose_pad;
2. connect_pad()
      case GST_AUTOPLUG_SELECT_EXPOSE:
        GST_DEBUG_OBJECT (dbin, "autoplug select requested expose");
        /* expose the pad, we don't have the source element */
        expose_pad (dbin, src, dpad, pad, caps, chain);
        res = TRUE;
        goto beach;

For the second case, video-sink set from app will not take effect. because in gstplaybin2.c:
1. autoplug_select_cb() will set group->video_sink with sink by auto-plug
2. in no_more_pads_cb(), the above sink will take a higher priority than sink set from app:
    if (group->video_sink) {
      GST_INFO_OBJECT (playbin, "setting custom video sink %" GST_PTR_FORMAT,
          group->video_sink);
      gst_play_sink_set_sink (playbin->playsink, GST_PLAY_SINK_TYPE_VIDEO,
          group->video_sink);
    } else {
      GST_INFO_OBJECT (playbin, "setting default video sink %" GST_PTR_FORMAT,
          playbin->video_sink);
      gst_play_sink_set_sink (playbin->playsink, GST_PLAY_SINK_TYPE_VIDEO,
          playbin->video_sink);
    }
}

In summary:
without 'video/x-raw-va' in DEFAULT_RAW_CAPS, application can't change 'video_sink' property of playbin2.

(In reply to comment #5)

> For the second case, video-sink set from app will not take effect. because in
> gstplaybin2.c:
> 1. autoplug_select_cb() will set group->video_sink with sink by auto-plug

That's exactly the problem. The sink *must* not be autoplugged by decodebin2/uridecodebin.

Could you attach the output of gst-inspect on your VA sink to this bug?

(In reply to comment #6)
> (In reply to comment #5)
> 
> > For the second case, video-sink set from app will not take effect. because in
> > gstplaybin2.c:
> > 1. autoplug_select_cb() will set group->video_sink with sink by auto-plug
> 
> That's exactly the problem. The sink *must* not be autoplugged by
> decodebin2/uridecodebin.
the sink is found by playbin2 in gstplaybin2.c, autoplug_select_cb().
not decodebin2/uridecodebin

Created attachment 166614 [details]
gst-inspect of MixVideoSink

now the caps shows as 'video/x-mixvideo-private', it will change to 'video/x-raw-va' soon.

(In reply to comment #5)
> For the second case, video-sink set from app will not take effect. because in
> gstplaybin2.c:
> 1. autoplug_select_cb() will set group->video_sink with sink by auto-plug
> 2. in no_more_pads_cb(), the above sink will take a higher priority than sink
> set from app:
>     if (group->video_sink) {
>       GST_INFO_OBJECT (playbin, "setting custom video sink %" GST_PTR_FORMAT,
>           group->video_sink);
>       gst_play_sink_set_sink (playbin->playsink, GST_PLAY_SINK_TYPE_VIDEO,
>           group->video_sink);
>     } else {
>       GST_INFO_OBJECT (playbin, "setting default video sink %" GST_PTR_FORMAT,
>           playbin->video_sink);
>       gst_play_sink_set_sink (playbin->playsink, GST_PLAY_SINK_TYPE_VIDEO,
>           playbin->video_sink);
>     }
> }
as to the above code.
if we take video_sink set from app as higher priority, it will also be ok for my case.

if(playbin->video_sink) {
gst_play_sink_set_sink (playbin->playsink, GST_PLAY_SINK_TYPE_VIDEO,
           playbin->video_sink);
} else {
gst_play_sink_set_sink (playbin->playsink, GST_PLAY_SINK_TYPE_VIDEO,
           group->video_sink);
}

is it reasonable?

Do your decoders use 'video/x-mixvideo-private' as the first format? Then everything should work. You should also use plabin2::flags|=GST_PLAY_FLAG_NATIVE_VIDEO to avoid converters being plugged.

my issue is "video/x-mxivideo-private" isn't exposed in decodebin2, not about conversions.
finally, I find another way to force decodebin2 expose "video/x-mixvideo-private": I try to find uridecodebin in app (playbin2's "element-added" signal), and add "video/x-mixvideo-private" to "caps" of uridecodebin in app.

Have you figured what criteria is causing decodebin2 to not expose your pads? I think it is somewhere in decodebin2:gst_decode_bin_expose().

you can refer to "comment 5" for more details.
decodebin2 doesn't think "video/x-raw-va"("video/x-mixvideo-private") is a raw caps to expose, next playbin2 find vaimagesink (MixVideoSink); then the pipeline is final. customized video sink set to playbin2 doesn't take effect.

Reopening as I think the requested information has been provided.

Could you try with latest GIT of core and base and get a debug log with debug level 5? With latest GIT this should really work if you set vaimagesink as custom video sink and it should even work if you don't set any custom video sink at all (playbin2 should select vaimagesink automatically then if it discovers the va caps).

I did some changes lately that improved the behaviour of playbin2 with custom caps that are supported by sinks (i.e. not the default raw caps).

Ok, let's close this as NOTABUG then. It should all work now and if it doesn't this should be fixed different than with a caps property on playbin2.