(In reply to Julian Sikorski from comment #0)
> Hello,
> 
> I just updated to git master (70eff01d) in order to check the status of bug
> 743687. Unfortunately, it is not possible to play anything:

That's odd. Which version of GStreamer are you using? 

Right now, using current master of GStreamer 1.5.0 (GIT), I'm able to run these pipelines:

gst-launch-1.0 playbin uri=http://samples.mplayerhq.hu/MPEG2/Futurama1-tmp.avi

gst-launch-1.0 playbin uri=http://samples.mplayerhq.hu/MPEG2/resolutionchange.mpg

Could you post a debug log with --gst-debug="vaapidecode:5,vaapipostproc:5,*:3"

It's with 1.4.5:

$ rpm -qa | grep gstreamer1 | sort
gstreamer1-1.4.5-1.fc21.i686
gstreamer1-1.4.5-1.fc21.x86_64
gstreamer1-debuginfo-1.4.5-1.fc21.x86_64
gstreamer1-devel-1.4.5-1.fc21.x86_64
gstreamer1-libav-1.4.3-1.fc21.x86_64
gstreamer1-plugins-bad-free-1.4.5-2.fc21.x86_64
gstreamer1-plugins-bad-freeworld-1.4.3-1.fc21.x86_64
gstreamer1-plugins-base-1.4.5-1.fc21.i686
gstreamer1-plugins-base-1.4.5-1.fc21.x86_64
gstreamer1-plugins-base-debuginfo-1.4.5-1.fc21.x86_64
gstreamer1-plugins-base-devel-1.4.5-1.fc21.x86_64
gstreamer1-plugins-good-1.4.5-2.fc21.x86_64
gstreamer1-plugins-ugly-1.4.3-1.fc21.x86_64
gstreamer1-vaapi-0.5.11.pre1-0.2.fc21.x86_64
libnice-gstreamer1-0.1.7-1.fc21.x86_64

$ LANG=C gst-launch-1.0 --gst-debug="vaapidecode:5,vaapipostproc:5,*:3"  playbin uri=http://samples.mplayerhq.hu/MPEG2/Futurama1-tmp.avi
Setting pipeline to PAUSED ...
Pipeline is PREROLLING ...
0:00:00.312314626 32373 0x7f031c0ec8a0 WARN                    alsa conf.c:4705:snd_config_expand: alsalib error: Unknown parameters {AES0 0x02 AES1 0x82 AES2 0x00 AES3 0x02}
0:00:00.312377310 32373 0x7f031c0ec8a0 WARN                    alsa pcm.c:2239:snd_pcm_open_noupdate: alsalib error: Unknown PCM default:{AES0 0x02 AES1 0x82 AES2 0x00 AES3 0x02}
libva info: VA-API version 0.36.0
libva info: va_getDriverName() returns 0
libva info: Trying to open /usr/lib64/dri/nvidia_drv_video.so
libva info: Found init function __vaDriverInit_0_36
libva info: va_openDriver() returns 0
Got context from element 'vaapidecode': gst.vaapi.Display=context, display=(GstVaapiDisplay)NULL;
0:00:00.400680060 32373 0x7f031c0daf70 WARN                GST_PADS gstpad.c:1058:gst_pad_set_active:<vaapipostproc:sink> Failed to activate pad
0:00:00.400723078 32373 0x7f031c0daf70 WARN               decodebin gstdecodebin2.c:2218:connect_pad:<decodebin0> Couldn't set vaapidecodebin0 to PAUSED
0:00:00.401140736 32373 0x7f031c0daf70 ERROR              decodebin gstdecodebin2.c:1503:analyze_new_pad:<decodebin0> New pad in a chain that is already complete
0:00:00.401654310 32373 0x7f031c0daf70 ERROR            vaapidecode gstvaapidecode.c:443:gst_vaapidecode_handle_frame:<vaapidecode0> not negotiated
0:00:00.422443898 32373      0x2313630 WARN                 basesrc gstbasesrc.c:2933:gst_base_src_loop:<source> error: Internal data flow error.
0:00:00.422487308 32373      0x2313630 WARN                 basesrc gstbasesrc.c:2933:gst_base_src_loop:<source> error: streaming task paused, reason not-negotiated (-4)
ERROR: from element /GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstSoupHTTPSrc:source: Internal data flow error.
Additional debug info:
gstbasesrc.c(2933): gst_base_src_loop (): /GstPlayBin:playbin0/GstURIDecodeBin:uridecodebin0/GstSoupHTTPSrc:source:
streaming task paused, reason not-negotiated (-4)
ERROR: pipeline doesn't want to preroll.
Setting pipeline to NULL ...
Freeing pipeline ...

I confirm it.

Compiled gstreamer-vaapi current master within a federa22 and the pipeline is not negotiated.

I will check with gstreamer 1.4 tomorrow.

Created attachment 304101 [details] [review]
workaround: enable vaapidecodebin only for gstreamer-1.5

Does this workaround work for you??

It does - I tested with 619058b. Doesn't it disable HW acceleration thought? I will try to bisect and find the change which caused the playback to break.

$ git bisect log 
git bisect start
# bad: [619058b5b705d927a36061aa5b952d7ab856d7e6] encoder: hevc: Fix the size over-flow for encoded buffer.
git bisect bad 619058b5b705d927a36061aa5b952d7ab856d7e6
# good: [3d8e5e59a72aafc5d06917a3b22006278697a7d7] vaapidecode: keep src caps and output state in sync
git bisect good 3d8e5e59a72aafc5d06917a3b22006278697a7d7
# skip: [c7f41b770b1076634f475e923c509116010ac369] codecparsers: Update to gst-vaapi-branch commit 43a0368
git bisect skip c7f41b770b1076634f475e923c509116010ac369
# skip: [2ddfcd8bc373e093d446c0f74ab52902932534cf] decoder: hevc: Fix decoding when there are RASL pictures present.
git bisect skip 2ddfcd8bc373e093d446c0f74ab52902932534cf
# good: [3f456df92c0750c98840d76ab5b2a93ae5432338] build: fix make distcheck
git bisect good 3f456df92c0750c98840d76ab5b2a93ae5432338
# good: [0c2fc4a381d30c4f18843fe9f610ccfde82b3524] Changing source code download links from https://gitorious  to https://github
git bisect good 0c2fc4a381d30c4f18843fe9f610ccfde82b3524
# bad: [100d5971b59b2a0b93510625b6addd98b0ab44bb] plugins: more specific log message
git bisect bad 100d5971b59b2a0b93510625b6addd98b0ab44bb
# good: [504fdedf84cc356d002539abac0b263363c9d4a8] vaapisink: use GstVideoSink vmethod show_frame()
git bisect good 504fdedf84cc356d002539abac0b263363c9d4a8
# bad: [7e08786e1d38bf98c3aba2ad6d3498f0dfca07d3] vaapidecode: Use the GstVideoDecoder error reporting function
git bisect bad 7e08786e1d38bf98c3aba2ad6d3498f0dfca07d3
# bad: [b3eccdb72be42469ad0d4fc45ffb6a8f7b3ebf25] decoder: hevc: cosmetics.
git bisect bad b3eccdb72be42469ad0d4fc45ffb6a8f7b3ebf25
# bad: [b5425da1de221b678c8819df543f2775f1cf1ad7] display: drm: fix race condition setting device type
git bisect bad b5425da1de221b678c8819df543f2775f1cf1ad7
# good: [0adb36b6587b099724453bc4ef0aafb1b65a392f] videopool: Release lock while allocating new object
git bisect good 0adb36b6587b099724453bc4ef0aafb1b65a392f
# bad: [6f298f6182614cfaf9f3d043207ebe8a6141ffaf] vaapipostproc: Don't create filter on caps query
git bisect bad 6f298f6182614cfaf9f3d043207ebe8a6141ffaf
# first bad commit: [6f298f6182614cfaf9f3d043207ebe8a6141ffaf] vaapipostproc: Don't create filter on caps query

(In reply to Julian Sikorski from comment #6)
> It does - I tested with 619058b. 

Cool

> Doesn't it disable HW acceleration thought?

It doesn't disable it. The workaround disables the compilation of vaapidecodebin, which is a bin composed by vaapidecode and vaapispostproc. Without it, the playbin uses vaapidecode instead.

> I will try to bisect and find the change which caused the playback to break.

As you see in your bisect, the issue is in vaapipostroc.

That patch defers the creation of the filter (thus the vaapi display) until _start(). But it also frees the allowed sink caps which may cause this issue in gstreamer 1.4

In gstreamer 1.5 it works OK.

I have just realized that this report is using the nvidia backend. I need to confirm it, but perhaps the nvidia backend doesn't support the vaapipostroc.

Similar one I think: https://bugzilla.gnome.org/show_bug.cgi?id=727841

Created attachment 306160 [details] [review]
vaapipostproc: return NULL caps if no filter available

THIS PATCH IS A WORK IN PROGRESS (don't apply)

This patch seems to do the trick with gstreamer 1.4 (decodebin tries
another encoder since vaapidecodebin fails to negotiate the sink caps)

But it raises a lot of awful warnings.

Review of attachment 306160 [details] [review]:

Why not return empty caps instead of NULL?

(In reply to Gwenole Beauchesne from comment #12)
> Review of attachment 306160 [details] [review] [review]:
> 
> Why not return empty caps instead of NULL?

I tried, but there are still warnings (less, but still).

I'm thinking that vaapidecodebin should be in charge when the vaapipostroc fail and don't leave that burden to decodebin.

Created attachment 306492 [details] [review]
vaapidecodebin: enable vpp if it is available

Instead of creating and adding VPP into the bin at setup, we wait until
we are sure the VA driver supports it. We know that when the VA video
context is received by the bin. Afterwards, it is decided to instanciate
and link the VPP or not.

This is more efficient and safer than waiting the VPP to fail and then
disable it.

Created attachment 306493 [details] [review]
vaapidecodebin: notify if vpp is disabled

When the system is aware that VPP is not available by the VA driver,
it would be useful to notify to the user that the disable-vpp property
has changed.

Both patches lgtm..

Did you manage to test the issue mentioned by the reporter??
I would appreciate if you can do some more testing with playbin to make sure that there is no regression..
make sure there is no issue for vaapidecodebin autoplugging in both 1.4 and 1.6,
no problem for falling back to software decoder if a particular profile is not supported by HW decoder,  test couple of of pipelines with "decodebin ! sink" combination etc...

Also what will happen if decoder produce a yuv format which is not supported by the vaapipostproc??

QA already started regression testing, but I would like to include this one also for 0.6...so we need to integrate it soon...

Once you did enough testing from your side, please feel free to push the patches..I will ask QA to restart the testing,

Thanks Victor :)

(In reply to sreerenj from comment #16)
> Both patches lgtm..
> 
> Also what will happen if decoder produce a yuv format which is not supported
> by the vaapipostproc??

Aha, stupid me.. This won't happen..forget about this comment :)

(In reply to sreerenj from comment #16)
> Both patches lgtm..

:O)

> 
> Did you manage to test the issue mentioned by the reporter??

Julian: ping?

Could you try these patches?

> I would appreciate if you can do some more testing with playbin to make sure
> that there is no regression..

So far:

HSW     gst-1.5.x OK
nVidia  gst-1.4.5 OK

> make sure there is no issue for vaapidecodebin autoplugging in both 1.4 and
> 1.6,
> no problem for falling back to software decoder if a particular profile is
> not supported by HW decoder,  test couple of of pipelines with "decodebin !
> sink" combination etc...

> QA already started regression testing, but I would like to include this one
> also for 0.6...so we need to integrate it soon...

Roger.

> 
> Once you did enough testing from your side, please feel free to push the
> patches..I will ask QA to restart the testing,
> 

Cool.

Older VA versions(0.32) needed this patch for sure... Otherwise gst-play-1.0 will simply fail. 
Another reason to push it for this release :)

HSW gst-1.4.5 OK

Pushing now.

Attachment 306492 [details] pushed as 5c799b3 - vaapidecodebin: enable vpp if it is available
Attachment 306493 [details] pushed as b4d9c0a - vaapidecodebin: notify if vpp is disabled

5c799b3 introduced regression:

The following pipeline is failing in Gst 1.4..
.... decodebin ! vaapipostproc ! vaapisink 

Reopening...

Created attachment 306946 [details] [review]
vaapidecodebin: use constructed() to configure bin

GObjectClass provides a constructed() virtual method which is a proper place
to define the bin's pipeline.

This patch moves the gst_vaapi_decode_bin_configure() to constructed() virtual
method.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Created attachment 306947 [details] [review]
vaapidecodebin: use pad templates in ghost pads

Instead of using the element's pad templates for creating the bin's ghost
pads, it is more coherent to use the bin's pad templates.

An unneeded cast was removed.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Created attachment 306948 [details] [review]
vaapidecodebin: post an error message if fails

If the construction of the bin fails, post an error message in the bus.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Created attachment 306949 [details] [review]
vaapidecodebin: add postprocessor dynamically

The former approach to left the bin unfinished has some problems, because we
could not assure that the bin was completely linked when the pipeline
started.

This approach uses the concepts of dynamic pipelines, as explained in
https://coaxion.net/blog/2014/01/gstreamer-dynamic-pipelines/

Initially the bin is fully functional, constructed as

(--------------------------------------------------)
|                vaapidecodebin                    |
|   (-------------)    (-------)    (----------)   |
|<--| vaapidecode |--->| queue |--->| identity |-->|
|   (-------------)    (-------)    (----------)   |
(--------------------------------------------------)

When the VaapiContext is shared and it has VPP capabilities, an idle pad probe
is launched. In the callback the bin is reconfigured dynamically, adding the
vaapipostproc element between the queue and the identity element:

(-----------------------------------------------------------------------)
|                                vaapidecodebin                         |
|   (-------------)    (-------)    (---------------)    (----------)   |
|<--| vaapidecode |--->| queue |--->| vaapipostproc |--->| identity |-->|
|   (-------------)    (-------)    (---------------)    (----------)   |
(-----------------------------------------------------------------------)

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Created attachment 306950 [details] [review]
vaapidecodebin: check for postproc instance

If the VPP's deinterlace-method is set, first we should check if the postproc
is already instanced to set it. Otherwise we just store it until the VPP is
added into the bin.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Created attachment 306951 [details] [review]
vaapidecodebin: remove postproc dynamically

Following the previous patch, this patch removes the VPP from the bin
dynamically, if it is already configured.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Created attachment 306998 [details]
pipeline_graph

Please see the attached pipeline graph for:
gst-launch-1.0 filesrc location=sample.mp4 ! decodebin ! vaapipostproc ! vaapisink 

There is only one vaapipostproc in the pipeline it seems!

Comment on attachment 306951 [details] [review]
vaapidecodebin: remove postproc dynamically

Testing this feature, the pipeline breaks. For now let's keep this feature as a TODO

(In reply to sreerenj from comment #29)
> Created attachment 306998 [details]
> pipeline_graph
> 
> Please see the attached pipeline graph for:
> gst-launch-1.0 filesrc location=sample.mp4 ! decodebin ! vaapipostproc !
> vaapisink 
> 
> There is only one vaapipostproc in the pipeline it seems!

The GST_MESSAGE_HAVE_CONTEXT never reach the vaapidecodebin's message handler when the outer vaapipostproc emits the VAAPI context.

I'm confused :S

Also these patches are quite non-trivial and may affect other playback scenarios too.For eg, we haven't tested loop-playback and all...
 
I am again proposing to disable vaapidecodebin for this release :)

Actually not disabling, we can decrease the RANK of vaapidecodebin so that vaapidecode will get autoplugged ahead of vaapidecodebin in normal playback cases(decodebin, playbin,,,)..

I agree that we lose the benefit of HW accelerated de-interlacing by default,  but still I prefer to be in the safe side by selecting vaapidecode as natural choice since it is the stable element...

We can increase the rank after this release so that we will get enough feedback/tests for vaapidecodebin by the time we release 0.7...
 
@Gwenole/@Victor ???

(In reply to sreerenj from comment #32)
> Also these patches are quite non-trivial and may affect other playback
> scenarios too.For eg, we haven't tested loop-playback and all...

Very true.

There's another alternative to these patches:

When vaapipostproc is instantiated, before adding it into the bin, it is set to PAUSED state. If it fails (after generating a VAAPI context for itself) it is ditched and the src ghost pad connects to the queue's src pad, otherwise it is returned to NULL state and linked in the bin.

The main drawback is that the instantiate of vaapidecodebin will take longer.

>  
> I am again proposing to disable vaapidecodebin for this release :)
> 
> Actually not disabling, we can decrease the RANK of vaapidecodebin so that
> vaapidecode will get autoplugged ahead of vaapidecodebin in normal playback
> cases(decodebin, playbin,,,)..

It sounds like a plan.


> I agree that we lose the benefit of HW accelerated de-interlacing by
> default,  but still I prefer to be in the safe side by selecting vaapidecode
> as natural choice since it is the stable element...
> 
> We can increase the rank after this release so that we will get enough
> feedback/tests for vaapidecodebin by the time we release 0.7...
>  
> @Gwenole/@Victor ???

commit 3ccb198b513dc6ad287fe44117d03bec4d6a966a
Author: Sreerenj Balachandran <sreerenj.balachandran@intel.com>
Date:   Tue Jul 7 20:57:20 2015 +0300

    Marking rank of vaapidecodebin as GST_RANK_MARGINAL for now.
    
    Unfortunately vaapidecodebin element is not seems to be stable
    enough for autoplugging ahead of vaapidecode.
    Lowering the rank for now (cosidering the immediate 0.6 release).
    
    See this: https://bugzilla.gnome.org/show_bug.cgi?id=749554
    
    Signed-off-by: Sreerenj Balachandran <sreerenj.balachandran@intel.com>

Comment on attachment 306946 [details] [review]
vaapidecodebin: use constructed() to configure bin

Let's postpone this a bit.

Created attachment 308863 [details] [review]
vaapidecodebin: has_vpp as a tri-state variable

has_vpp can be UNKNOWN while the context message hasn't being received.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Created attachment 308864 [details] [review]
vaapidecodebin: post an error message if fails

If the construction of the bin fails, post an error message in the bus.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Created attachment 308865 [details] [review]
vaapidecodebin: ensure VPP before going to READY

There are sometimes that the VA-API display context is not shared among the
pipeline, but it is important to know it before going to READY state (when the
pipeline is already linked).

One instance of this case is this:

gst-launch-1.0 filesrc location=media ! decodebin ! vaapipostproc ! vaapisink

This patch adds a new function in gstvaapipluginutil called
gst_vaapi_create_test_display(). Its purpose is to create a disposable VA-API
display, which only will be used for verify if the VAEntrypointVideoProc is
available by the hardware. Afterwards, it should be unrefed.

If the vaapidecodebin is going to READY state, and the element still doesn't
know if VPP is available, the last resort is to create a new instance of the
VA-API display and test for it.

Created attachment 308866 [details] [review]
vaapidecodebin: check for postproc instance

If the VPP's deinterlace-method is set, first we should check if the postproc
is already instanced to set it. Otherwise we just store it until the VPP is
added into the bin.

Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>

Attachment 308863 [details] pushed as 0004301 - vaapidecodebin: has_vpp as a tri-state variable
Attachment 308864 [details] pushed as 1e061c5 - vaapidecodebin: post an error message if fails
Attachment 308865 [details] pushed as ee5d8ee - vaapidecodebin: ensure VPP before going to READY
Attachment 308866 [details] pushed as 7f34c6e - vaapidecodebin: check for postproc instance