On Thu, Feb 28, 2013 at 05:53:49PM +0100, Max Dm wrote:
> On Thu, Feb 28, 2013 at 3:16 PM, Diego Biurrun <[email protected]> wrote:
> > On Thu, Feb 28, 2013 at 02:47:39PM +0100, Max Dm wrote:
> > > On Thu, Feb 28, 2013 at 11:38 AM, Diego Biurrun <[email protected]>
> > wrote:
> > > > On Wed, Feb 27, 2013 at 11:46:45PM +0100, Maxym Dmytrychenko wrote:
> > > > > On 13-02-27 19:50:51, Diego Biurrun wrote:
> > > > > > On Wed, Feb 27, 2013 at 06:39:57PM +0100, maxd wrote:
> > > > > > > On 13-02-21 16:03:30, Diego Biurrun wrote:
> > > > > > > > On Mon, Feb 04, 2013 at 08:49:35PM +0100, Maxym Dmytrychenko
> > wrote:
> > > > > > > > > --- /dev/null
> > > > > > > > > +++ b/libavcodec/qsv.h
> > > > > > > > > @@ -0,0 +1,469 @@
> > > > > > > > You start the sentence with "As ..." but then you do not say
> > what
> > > > follows
> > > > > > > > from the ability to use hw acceleration.
> > > > > > > >
> > > > > > > > > +#ifdef HAVE_AV_CONFIG_H
> > > > > > > > > +#include "config.h"
> > > > > > > > > +#endif
> > > > > > > >
> > > > > > > > No such inclusion guard is necessary.
> > > > > > > it looks that might help to the final application ( dependency
> > point
> > > > as
> > > > > > > it was last time )
> > > > > > > I can check more here if needed.
> > > > > > > If I will remove it from this file - will be a transparent change
> > > > > > > for QSV implementation inside libav and final application.
> > > > > >
> > > > > > You cannot include config.h in an installed header, with or
> > without the
> > > > > > inclusion guards.
> > > > >
> > > > > ok, if to move into qsv.c - will it be fine ?
> > > > > if so, next patch will move lines:
> > > > >
> > > > > #ifdef HAVE_AV_CONFIG_H
> > > > > #include "config.h"
> > > > > #endif
> > > >
> > > > Again - no such inclusion guard is necessary.
> > > >
> > > > right - it was just to show the idea and what is moved,
> > > #ifdef HAVE_AV_CONFIG_H will be dropped while in qsv.c
> >
> > It has to be and had to be dropped anywhere.
> >
> > > Again,  will such move help to solve the question?
> >
> > Not sure which question you are referring to.
> >
> > question about having config.h included
> I think we have the agreement : if moved to qsv.c with no HAVE_AV_CONFIG_H
> - fine,
> correct?

Probably yes.

> > > you can have a look on MediaSDK samples and definitions been used there,
> > > files like sample_common\include\sample_defs.h
> >
> > URL?
> >
> Homepage of MediaSDK is
> http://software.intel.com/en-us/vcsource/tools/media-sdk
> I can recommend to download and install MediaSDK to see this file.

I cannot be bothered to be honest.  There is no git/whatever tree directly
visible online?

> please note:
> regardless the fact that this, an initial patch provides QSV-based decode
> only - it has already "looking forward" structure and implementation.
> 
> QSV based acceleration (and MediaSDK API ) goes beyond decode and only,
> it covers filters/VPP cases and encode - meaning: full video transcode
> scenario.
> I would encourage to read more from :
> http://software.intel.com/en-us/vcsource/tools/media-sdk
> and provided together with MediaSDK documentations.
> 
> if an application does video transcode or any decode/encode : for best
> performance with QSV - we have to recommend to use not only libav/QSV
> decode but QSV based encode/filters as well.
> Such, fully HW accelerated, approach probably something new for libav but
> provided patch and its definitions and structures is a start to build such
> possibility as within libav and up to the final application.
> Note, it goes beyond libav at the level of final application.
> This is what I mean by "looking forward".
> 
> About possible concern: to have only needed definitions etc - just believe
> that it will stay so but just extended in the future.

Let's add the extra parts when we have a need for them, not earlier.
We have had a long history of features added for future use and then
left to rot.

Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to