[Bug 754291] New: Compositor fails with "reason not-negotiated" when changing the pixel-aspect-ratio during runtime

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Bug ID: 754291
Summary: Compositor fails with "reason not-negotiated" when
changing the pixel-aspect-ratio during runtime
Classification: Platform
Product: GStreamer
Version: git master
OS: Linux
Status: NEW
Severity: normal
Priority: Normal
Component: gst-plugins-bad
Assignee: gstreamer-***@lists.freedesktop.org
Reporter: ***@mazdermind.de
QA Contact: gstreamer-***@lists.freedesktop.org
GNOME version: ---

Created attachment 310267
--> https://bugzilla.gnome.org/attachment.cgi?id=310267&action=edit
example program

The test-program attached to this bug contains a pipeline like this:

`
videotestsrc pattern=red !
video/x-raw,width=1920,height=1080,format=I420,framerate=25/1,pixel-aspect-ratio=1/1
!
videoscale !
capsfilter name=caps0 caps=video/x-raw !
mix.

compositor name=mix !
video/x-raw,width=1920,height=1080,format=I420,framerate=25/1,pixel-aspect-ratio=1/1
!
xvimagesink
`

The caps property of the capsfilter caps0 is, 1 second after the pipeline
starts, set to `video/x-raw,width=950,height=534`

Because the requested resolution 950:534 is not exactly 16:9 as the input, the
videoscaler will produce videoframes with a PAR of 1424/1425

`
video/x-raw, format=(string)I420, width=(int)950, height=(int)534,
framerate=(fraction)25/1, pixel-aspect-ratio=(fraction)1424/1425,
interlace-mode=(string)progressive
`
The compositor doesn't allow this change and terminates the pipeline.

Using videomixer instead of compositor does not show this behavior and forcing
a pixel-aspect-ratio of 1/1 between the videoscaler and the compositor also
fixes the issue.

What worries me is that this behavior was not visible in 1.5.1

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Nirbheek Chauhan <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #310267|text/x-python |text/plain
mime type| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@coaxion.net
Target Milestone|git master |1.5.91
Severity|normal |blocker

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #1 from Peter Körner <***@mazdermind.de> ---
Created attachment 310334
--> https://bugzilla.gnome.org/attachment.cgi?id=310334&action=edit
logfile of the compositor failing (level 4)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|1.5.91 |1.6.0

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Nirbheek Chauhan <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@gmail.com

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Thiago Sousa Santos <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@gmail.com

--- Comment #2 from Thiago Sousa Santos <***@gmail.com> ---
Does compositor support renegotiation? It seems since 1.5 that videoaggregator
(the base class) denies negotiation if the format of pixel-aspect-ratio are
different.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #3 from Sebastian Dröge (slomo) <***@coaxion.net> ---
It should still allow different format and par. compositor and glvideomixer
both can convert between all possible raw formats.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #4 from Thiago Sousa Santos <***@gmail.com> ---
(In reply to Sebastian Dröge (slomo) from comment #3)

Post by "GStreamer" (GNOME Bugzilla)
It should still allow different format and par. compositor and glvideomixer
both can convert between all possible raw formats.

Yes, but is this a blocker?

@Peter: Does it work just by reverting gst-plugins-bad to 1.5.1? Or do you need
to revert all modules?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #5 from Sebastian Dröge (slomo) <***@coaxion.net> ---
(In reply to Thiago Sousa Santos from comment #4)

Post by "GStreamer" (GNOME Bugzilla)
(In reply to Sebastian Dröge (slomo) from comment #3)

Post by "GStreamer" (GNOME Bugzilla)
It should still allow different format and par. compositor and glvideomixer
both can convert between all possible raw formats.

Yes, but is this a blocker?

The bug report says that it worked in 1.5.1, so I would consider it a blocker
:)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #6 from Thiago Sousa Santos <***@gmail.com> ---
Reverting -bad to 1.5.1 works, will try git bisect.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #7 from Thiago Sousa Santos <***@gmail.com> ---
commit c28d90de68cd07a54c9bc21a5f1fde619dc81c55
Author: Thibault Saunier <***@gnome.org>
Date: Mon Jul 6 18:51:07 2015 +0200

videoaggregator: Remove pixel-aspect-ratio field from the caps returned by
getcaps

Avoiding not negotiated error while negotiating as we anyway force 1/1
as output

Bisected to this one, the difference from 1.5.1 to master is that the
negotiation in 1.5.1 would be to p-a-r=1/1 and in master is to 1424/1425, which
is rejected*


* but actually tries to go and asserts ever since
commit 6efc106a67ce6ba2a8313150ef919c0be9234da6
Author: Olivier Crête <***@collabora.com>
Date: Fri Mar 6 19:50:08 2015 -0500

aggregator: Queue "latency" buffers at each sink pad.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #8 from Thiago Sousa Santos <***@gmail.com> ---
Created attachment 310515
--> https://bugzilla.gnome.org/attachment.cgi?id=310515&action=edit
videoaggregator: removing restriction on caps format changes

videoaggregator was not allowing format, PAR or interlace mode
changes in caps, this patch lifts this restriction

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Thiago Sousa Santos <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Summary|Compositor fails with |videoaggregator: Compositor
|"reason not-negotiated" |fails with "reason
|when changing the |not-negotiated" when
|pixel-aspect-ratio during |changing the
|runtime |pixel-aspect-ratio during
| |runtime

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #310515|none |needs-work
status| |

--- Comment #9 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Comment on attachment 310515
--> https://bugzilla.gnome.org/attachment.cgi?id=310515
videoaggregator: removing restriction on caps format changes

interlace-mode changes are still not allowed. It can't interlace or deinterlace
videos, so would composite interlaced and progressive videos into the same
output.

Otherwise this makes sense

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #10 from Thiago Sousa Santos <***@gmail.com> ---
It seems to be lacking some proper restriction on that. The getcaps seems to
report correctly the current interlace-mode but you could have 2 sink pads
having its inputs being set to interlaced and non-interlaced before the source
pad gets configured, leading to some inconsistency there.

Do we want to fix that here or let's postpone that to post-1.6 and just do the
simple fix with a variation of the proposed patch?

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #11 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Is the interlacing part easy/non-intrusive to fix? If not, keep it as is and
just fix the PAR problem :)

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #12 from Thiago Sousa Santos <***@gmail.com> ---
Created attachment 310555
--> https://bugzilla.gnome.org/attachment.cgi?id=310555&action=edit
videoaggregator: lift restriction of changing format and pixel-aspect-ratio

The videoaggregator can convert format and PAR, there is no apparent
reason for restricting it.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

--- Comment #13 from Thiago Sousa Santos <***@gmail.com> ---
Seems to require some locking and no one complained, I'd defer it to after 1.6
unless someone else wants to fix it now.

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Thiago Sousa Santos <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Blocks| |754495

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #310555|none |accepted-commit_now
status| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Thiago Sousa Santos <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED

--- Comment #14 from Thiago Sousa Santos <***@gmail.com> ---
commit 8c5b2da124ebe5cf68eaca758dd40c92ab6228b7
Author: Thiago Santos <***@osg.samsung.com>
Date: Wed Sep 2 19:16:56 2015 -0300

videoaggregator: lift restriction of changing pixel-aspect-ratio

The videoaggregator can convert PAR, there is no reason for
restricting it.

https://bugzilla.gnome.org/show_bug.cgi?id=754291

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Thiago Sousa Santos <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #310555|accepted-commit_now |committed
status| |

["GStreamer" (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=754291

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|1.6.0 |1.5.91