Lets keep only 1.6 branch then...
Who ever want to use older 1.4 should take the gstremaer-vaapi-0.7.x  :)

(In reply to sreerenj from comment #1)
> Lets keep only 1.6 branch then...
> Who ever want to use older 1.4 should take the gstremaer-vaapi-0.7.x  :)

I mean, master branch and 1.6....

(In reply to sreerenj from comment #2)
> (In reply to sreerenj from comment #1)
> > Lets keep only 1.6 branch then...
> > Who ever want to use older 1.4 should take the gstremaer-vaapi-0.7.x  :)
> 
> I mean, master branch and 1.6....

D'accord!

Regarding the codecparsers, only vp9 parser is missing in upstream.

Right now I am merging all vp9 parser patches(from gstreamer-codecparsers) to a single one for upstreaming, hope we can push it if there is no issue for other developers... It is in good shape I think :)

There is only one extra patch in vaapiparse_h264 which is missing in upstream (all others are build specific patches for different GST versions).
The rebased patch is ready to push here:
https://bugzilla.gnome.org/show_bug.cgi?id=732167

Why is bug #732167 a requirement for gstreamer-vaapi?

If you want your input buffers to have a particular format, you should just advertise that requirement on your input pad template and then h264parse should do the right thing.

What h264parse outputs by default should not be of any concern to any decoder really, unless I'm missing something.

I have a branch in my personal repo with these tasks:

https://github.com/ceyusa/gstreamer-vaapi/commits/759192

(In reply to Tim-Philipp Müller from comment #6)
> Why is bug #732167 a requirement for gstreamer-vaapi?
> 
> If you want your input buffers to have a particular format, you should just
> advertise that requirement on your input pad template and then h264parse
> should do the right thing.
> 
> What h264parse outputs by default should not be of any concern to any
> decoder really, unless I'm missing something.

vaapidecode can work with any combination.. I have add #732167 just for tracking..vaapiparse_h264 is the part of gstremaer-vaapi, we are removing those from source and all patches in gstremaer-vaapi/patches/videoparsers ....
So in  theory, any patch we remove from patches/videoparsers should land in upstream ;)

Ok, as I see it that patch is entirely gratuitious and we already have a bug to bikeshed about its merits, but it doesn't block gstreamer-vaapi upstreaming then.

I have a couple patches more in my branch https://github.com/ceyusa/gstreamer-vaapi/commits/759192

* Remove old gst version guards
* Remove video parser crufts

Also I think we should simplify our configure.ac, even more, we might align with gstreamer's way, using its common submodule. It doesn't look that hard. I'll give it a try today.

Created attachment 319678 [details] [review]
Remove debian.upstream packaging

Created attachment 319679 [details] [review]
docs: remove library documentation which is non-public now

Comment on attachment 319678 [details] [review]
Remove debian.upstream packaging

debian.upstream has been useful to me to test upstream in a debian schroot, but I agree, it doesn't belong to the project itself.

Comment on attachment 319679 [details] [review]
docs: remove library documentation which is non-public now

Yeah, I was reluctant to do this, because that info is useful to me, and we have spent some time keeping the docs more or less in shape (or at least building).

My plan is to bring back the libgstvaapi, but only exposing GstVaapDisplay because it is useful for the user to set their own VaapiDisplay (like GstGLContext). So, we would need to revert part of this.

But, yes, I agree on remove these docs right now.

I'm emerging from the autotool's dungeons, and I would like to think that I came out victorious (that's my name ;))

In my personal repo

https://github.com/ceyusa/gstreamer-vaapi/commits/759192

I have 26 new patches from the current upstream repo that put in order our autotool house, integrate the use of the gst-common submodule and fix the plugin's gtk-doc generation.

In the gtk-doc generation I found an issue: because the encoders name have the pattern vaapiencode_{codec} the documentation of the vaapi plugin can link those pages because it assumes that the _ is changed to -

Anyway, it is a minor issue which we could track later. Perhaps we should rename the encoders to follow the common pattern in gstreamer.

Before merging: those commits are thinking in GStreamer 1.7. I need to verify 1.6 and branch.

> I'm emerging from the autotool's dungeons, and I would like to
> think that I came out victorious (that's my name ;))

:D (You may have won the battle, but will you win the war? ;))

Ok, let me know when it's ok to push.

Just on a side note, re. the commit message, the reason we have our own gtk-doc thingy is not because gtk-doc is no longer maintained, but because we need to do some very GStreamer-specific things there (gtk-doc is made for stuff that has public API, which our plugins don't have).

> In the gtk-doc generation I found an issue: because the encoders name have
> the pattern vaapiencode_{codec} the documentation of the vaapi plugin can
> link those pages because it assumes that the _ is changed to -
> 
> Anyway, it is a minor issue which we could track later. Perhaps we should
> rename the encoders to follow the common pattern in gstreamer.

I think the common pattern would be something like vaapih264dec / vaapih264enc or vah264dec / vah264enc or such?

Created attachment 320179 [details] [review]
vaapi: fix 'ISO C90 forbids mixed declarations and code' compiler warnings

Needed this when building against master with your branch.

Created attachment 320180 [details] [review]
Fix some more compiler warning

Two (false) compiler warnings about variables potentially
being used uninitialized, and one about a variable being
set but not used.

One more thing I noticed: ./autogen.sh now installs the local pre-commit hook to check for GStreamer-style indentation with GNU indent.

The entire code base has indentation that's inconsistent with the GStreamer indentation though, try: gst-indent gst-libs/gst/vaapi/*.c gst/vaapi/*.c

How do you want to handle this?

Options:

a) one commit to reindent everything

b) git filter-branch which re-indents the entire history (this way everything will be tidy from the start and history won't be 'disturbed' by a global reindentation commit, downside is the history is then not a neat continuation on top of the old github repository [it is of course, but it won't look like that to git])

c) carry on with custom indentation (my least favourite option)

In an act of boldness I decide to move this commit to the new gstreamer-vaapi component in GStreamer o/ 

(In reply to Tim-Philipp Müller from comment #17)
> > I'm emerging from the autotool's dungeons, and I would like to
> > think that I came out victorious (that's my name ;))
> 
> :D (You may have won the battle, but will you win the war? ;))

We're all doomed!! XD

> Ok, let me know when it's ok to push.

Yeah, just realized that is broken with gst1.6, because there's no vp9 parser. I just need to bring back the vp9 parser check in configure.ac.

> Just on a side note, re. the commit message, the reason we have our own
> gtk-doc thingy is not because gtk-doc is no longer maintained, but because
> we need to do some very GStreamer-specific things there (gtk-doc is made for
> stuff that has public API, which our plugins don't have).

Yes, I know that, but I guess I failed at my wording in the commit message. I'll fix that.

> > In the gtk-doc generation I found an issue: because the encoders name have
> > the pattern vaapiencode_{codec} the documentation of the vaapi plugin can
> > link those pages because it assumes that the _ is changed to -
> > 
> > Anyway, it is a minor issue which we could track later. Perhaps we should
> > rename the encoders to follow the common pattern in gstreamer.
> 
> I think the common pattern would be something like vaapih264dec /
> vaapih264enc or vah264dec / vah264enc or such?

I'd vote for vaapi{codec}enc

(In reply to Tim-Philipp Müller from comment #18)
> Created attachment 320179 [details] [review] [review]
> vaapi: fix 'ISO C90 forbids mixed declarations and code' compiler warnings
> 
> Needed this when building against master with your branch.

(In reply to Tim-Philipp Müller from comment #19)
> Created attachment 320180 [details] [review] [review]
> Fix some more compiler warning
> 
> Two (false) compiler warnings about variables potentially
> being used uninitialized, and one about a variable being
> set but not used.

I usually compile with clang and I never saw those complains :/

(In reply to Tim-Philipp Müller from comment #20)
> One more thing I noticed: ./autogen.sh now installs the local pre-commit
> hook to check for GStreamer-style indentation with GNU indent.
> 
> The entire code base has indentation that's inconsistent with the GStreamer
> indentation though, try: gst-indent gst-libs/gst/vaapi/*.c gst/vaapi/*.c
> 
> How do you want to handle this?
> 
> Options:
> 
> a) one commit to reindent everything
> 
> b) git filter-branch which re-indents the entire history (this way
> everything will be tidy from the start and history won't be 'disturbed' by a
> global reindentation commit, downside is the history is then not a neat
> continuation on top of the old github repository [it is of course, but it
> won't look like that to git])
> 
> c) carry on with custom indentation (my least favourite option)

I'd change all by once in gst/vaapi/*.c since there were only a couple errors in the indentation.

In the case of gst-libs/vaapi/*.c the changes would be massive. I don't know.

Sree, what do you think?

> I usually compile with clang and I never saw those complains :/

The GStreamer autotools infrastructure will add -Wdeclaration-after-statement to WARNING_CFLAGS if supported by the compiler (we target C89 :))

(In reply to Víctor Manuel Jáquez Leal from comment #23)
> (In reply to Tim-Philipp Müller from comment #20)
> > One more thing I noticed: ./autogen.sh now installs the local pre-commit
> > hook to check for GStreamer-style indentation with GNU indent.
> > 
> > The entire code base has indentation that's inconsistent with the GStreamer
> > indentation though, try: gst-indent gst-libs/gst/vaapi/*.c gst/vaapi/*.c
> > 
> > How do you want to handle this?
> > 
> > Options:
> > 
> > a) one commit to reindent everything
> > 
> > b) git filter-branch which re-indents the entire history (this way
> > everything will be tidy from the start and history won't be 'disturbed' by a
> > global reindentation commit, downside is the history is then not a neat
> > continuation on top of the old github repository [it is of course, but it
> > won't look like that to git])
> > 
> > c) carry on with custom indentation (my least favourite option)
> 
> I'd change all by once in gst/vaapi/*.c since there were only a couple
> errors in the indentation.
> 
> In the case of gst-libs/vaapi/*.c the changes would be massive. I don't know.
> 
> Sree, what do you think?

Just do a massive gst-indent in gst-libs/gst/vaapi,, only problem is that we have to rewrite the pending patches in bugzilla for merging ;)

BTW, i am still pushing few vp9decode patches in github,,,would be great if you can rebase those before indenting...

(In reply to Víctor Manuel Jáquez Leal from comment #21)

> 
> Yeah, just realized that is broken with gst1.6, because there's no vp9
> parser. I just need to bring back the vp9 parser check in config
> 
> > > In the gtk-doc generation I found an issue: because the encoders name have
> > > the pattern vaapiencode_{codec} the documentation of the vaapi plugin can
> > > link those pages because it assumes that the _ is changed to -
> > > 
> > > Anyway, it is a minor issue which we could track later. Perhaps we should
> > > rename the encoders to follow the common pattern in gstreamer.
> > 
> > I think the common pattern would be something like vaapih264dec /
> > vaapih264enc or vah264dec / vah264enc or such?
> 
> I'd vote for vaapi{codec}enc

We had a plan to do  the splitting for decoders like the encoders..
(Unlike encoders, we have a common vaapidecode for whole the codecs instead of vaapih264dec, vaapivp9dec etc)
https://bugzilla.gnome.org/show_bug.cgi?id=734093

Renaming of elements will create bit of confusion, but if we are planning for a name change, then this is the right time for sure :)
vaapi{codec}dec/vaapi{codec}enc is fine for me...

I think We can push once the 1.6 branch is ready :)

So, this is my plan:

1) To merge my bunch of patches which re-organizing our autotools

* 8c3a4a6 plugins: use the same pre-processor macro
* 706b26b decoder: update a deprecated function
* c753929 decoder: fix code style
* 7ea4114 build: fix the use of configure's cache
* 065d46d build: use common version variables
* df78b8c build: hard-code an unneeded macro
* 612b01b build: refactorization of dependency tracking
* 3613100 build: check for OpenGL either GLX or EGL are requested
* f7cd55f build: indent and add square braces
* 88a0eeb build: upgrade autotools version dependency
* cc613d6 build: enhance string comparisons
* 448ce65 build: remove unused variables
* 6225bc6 build: remove check for old version of gstreamer
* 32ffb8a build: remove GStreamer's parsers checks 
         (TODO: don't remove the VP9 check, since it is not merged in gst-plugins-bad 1.6)
* 3c6eb03 build: add gstreamer-pbutils dependency
* 1ca1403 build: fix variable declaration
* 54011c2 build: fix when HEVC decoder is disabled
* ba91bf4 build: remove unused EGL specific sources
* 14662ee build: remove check for GStreamer 1.2
* e387ad5 Remove more video parser crufts

2) To merge the latest patches from Sree in master:

* b072a10 decoder: vp9: Fix crop rectangle setting
* 17412c3 vaapidecode: Fix renegotiation for resolution change

3) To merge Tim's patches

* 41b1db3 Fix some more compiler warning
* a85df93 vaapi: fix 'ISO C90 forbids mixed declarations and code' compiler warnings

3) To merge the re-indentation mega-patch

4) merge the gst-common integration patches (aiming to branch 1.6 of gstreamer-common)

* 11a62d1 docs: update plugin documentation
* eed5bce vaapidecodebin: fix code style
* 7cf7892 resurrect gtk-doc machinery
          (TODO: reword this commit log)
* 8a5d628 use gst-common submodule
* 0c5d4ab add gst-common submodule
          (TODO: point to its branch 1.6)
* d6e8f54 add doap descriptor

5) Bump version to ===> 1.6.0 <===

   From here we could branch out for fixes in this "stable" release, which is the same of 0.7.0

6) Bump version to ===> 1.7.0 <=== 
    
7) update gst-common submodule to master
   enable VP9 decoding

8) Rename the encoders
   
9) Back to fix stuff, moving bugs to the new bugzilla's place

Do we agree?

These two

* c753929 decoder: fix code style
* eed5bce vaapidecodebin: fix code style

Shall be merged into the re-indent mega patch.

+1

+1

A note about the split decoder, I don't split in v4l2 right now either. But when we'll start adding non-parsing decoders in there, it might become a good idea.

Uff!... done.

Everything is in place as we agreed in comment #28

https://github.com/ceyusa/gstreamer-vaapi/commits/759192

I have created an annotated tag 1.6.0 with my gpg key.

hold me a sec! clang did it again... fixing

now the branch should be singing and dancing :)

Any reason for keeping 1.6.3 as min requirement in 1.6 branch?
I prefer 1.6.0 as minimum requirement...

otherwise LGTM, rest we can fix directly in upstream,,,
Now it Tim's call to push :)

Done, and git commit hooks hooked up.

> I have created an annotated tag 1.6.0 with my gpg key.

Weird thing though, if I do 'git describe' it still shows 0.7.0-X instead of 1.6.0-X, not sure why.

Not in http://cgit.freedesktop.org/gstreamer/  yet? :)

I was going to say 'it might take a day or so to show up', but it seems to be there now :)

http://cgit.freedesktop.org/gstreamer/gstreamer-vaapi/

Great !