[Poll] Expose present surface object as in Framegrabber as event?

Discussion in 'General Development (no feature request here!)' started by gemx, January 13, 2010.

?

Add the patch to SVN?

  1. Yup, can't do any harm

    57.1%
  2. Nope (please give a comment)

    42.9%
  1. gemx
    • Team MediaPortal

    gemx Retired Team Member

    Joined:
    October 31, 2006
    Messages:
    1,972
    Likes Received:
    541
    Ratings:
    +541 / 0
    Home Country:
    Germany Germany
    Show System Specs
    Hi,

    while working on the Atmolight plugin (see here https://forum.team-mediaportal.com/...in-update-13-01-2010-v1-1-a-75840/#post557386) i noticed that the current method in the framebuffer always copies the whole image from the GPU which is a big CPU eater.
    Even more when you think about 1080i/p video content.
    In most cases it is enough to get only a small portion of the screen (e.g. for black bar detection) or get a smaller, resized image like e.g. in my case.

    I tried many different ways to capture the current screen content but the framegrabber was the only solution since it gets feeded directly by PlaneSurface.PresentImage(...).

    To give Plugin developers the freedom of choice how they would like the current image i made the attached small patch.
    It just exposes an OnNewFrameEvent from the framegrabber, where the current image is handed over before it's processed by the framegrabber itself.

    It's just a very small patch and until this event is not hooked to any code it won't be used anyway but maybe of great help to take CPU load from the framegrabber.

    Ok, I know, we have a feature freeze but this is not really a feature - it helps to reduce CPU load in MP if used by a plugin and if not it doesn't have any effect or side effect at all.

    This is a bit like begging - but please vote for "yes" because otherwise my (and probably that of many other) Atmolight Hardware will never run smooth with MP :p

    So, what do you think?

    Please vote :)

    :D

    The patch is here
    Code (Text):
    1.  
    2. Index: Core/Player/FrameGrabber.cs
    3. ===================================================================
    4. --- Core/Player/FrameGrabber.cs    (revision 24717)
    5. +++ Core/Player/FrameGrabber.cs    (working copy)
    6. @@ -45,6 +45,9 @@
    7.  
    8.      private FrameGrabber() {}
    9.  
    10. +    public delegate void NewFrameHandler(Int16 width, Int16 height, Int16 arWidth, Int16 arHeight, uint pSurface);
    11. +    public event NewFrameHandler OnNewFrame;
    12. +
    13.      public static FrameGrabber GetInstance()
    14.      {
    15.        if (instance == null)
    16. @@ -120,6 +123,16 @@
    17.      /// <param name="pSurface"></param>
    18.      public void OnFrame(Int16 width, Int16 height, Int16 arWidth, Int16 arHeight, uint pSurface)
    19.      {
    20. +      if (OnNewFrame != null)
    21. +      {
    22. +        try
    23. +        {
    24. +          OnNewFrame(width, height, arWidth, arHeight, pSurface);
    25. +        }
    26. +        catch(Exception)
    27. +        {
    28. +        }
    29. +      }
    30.        // Is GetCurrentImage() requesting a frame grab?
    31.        if (!grabSample)
    32.        {
    33.  



     
  2. Google AdSense Guest Advertisement



    to hide all adverts.
  3. tourettes
    • Team MediaPortal

    tourettes Retired Team Member

    Joined:
    January 7, 2005
    Messages:
    17,301
    Likes Received:
    4,595
    Ratings:
    +4,810 / 3
    Just wondering if those automatic zoom mode plugins are experiencing from high CPU usage as well? In that case I would call it as a bug fix. But somehow I feel like I'm not able to vote as I have lobbyed such huge amount for the feature freeze :)
     
  4. gemx
    • Team MediaPortal

    gemx Retired Team Member

    Joined:
    October 31, 2006
    Messages:
    1,972
    Likes Received:
    541
    Ratings:
    +541 / 0
    Home Country:
    Germany Germany
    Show System Specs
    Well, kind of. IFC plugin for example only calls the framegrabber once per second but that could nevertheless cause a CPU peak when watching HD content which could stall MP for some ms.
    Althought this doesn't sound much, just think about these timing issues we had lately with the EVR renderer. ;)

    I don't say that this caused this behaviour but there is a possibility that it could cause this or other timing issues
     
  5. tourettes
    • Team MediaPortal

    tourettes Retired Team Member

    Joined:
    January 7, 2005
    Messages:
    17,301
    Likes Received:
    4,595
    Ratings:
    +4,810 / 3
    Yes, it would indeed cause same stuttering (only with smaller frequency).
     
  6. arion_p
    • Team MediaPortal

    arion_p Retired Team Member

    Joined:
    February 7, 2007
    Messages:
    3,352
    Likes Received:
    1,447
    Occupation:
    Developer
    Location:
    Athens
    Ratings:
    +1,522 / 0
    Home Country:
    Greece Greece
    Show System Specs
    I think this has been discussed before. While I agree that this is a welcome improvement, it may also cause issues if the event handler takes too long. IIRC this is called from within PresentImage(), so a long running event handler could easily hinder rendering and cause stuttering. I believe we need to somehow limit how long the event handler will take to finish.

    Even though the event handler provides the ultimate flexibility it introduces a big risk. Perhaps instead of an event handler we could provide a limited set of frame pre-processing options that are known to be fast and safe.

    Just some thoughts ;)
     
  7. gemx
    • Team MediaPortal

    gemx Retired Team Member

    Joined:
    October 31, 2006
    Messages:
    1,972
    Likes Received:
    541
    Ratings:
    +541 / 0
    Home Country:
    Germany Germany
    Show System Specs
    I already thought about something like "GetCurrentImage(Rectangle source,Rectangle target)" which would work but the problem with this is that you have to specify the dimensions when in OnFrame(...) which would result in every application that uses the GetCurrentImage(...) would get the same regions or worse different Rect request would be overriden by each other.

    So the only solution i found was to deliver the source surface untouched in the event handler so that every function that registered to it can make it's own resizing but that would on the other hand mean the might happen several resizes (copies from GPU -> CPU) for the same surface which is also bad.

    Best intermediate solution would be to always resize the image to something like 300*200 (keeping the AR) and then output it via GetCurrentImage.

    How about that?
     
  8. arion_p
    • Team MediaPortal

    arion_p Retired Team Member

    Joined:
    February 7, 2007
    Messages:
    3,352
    Likes Received:
    1,447
    Occupation:
    Developer
    Location:
    Athens
    Ratings:
    +1,522 / 0
    Home Country:
    Greece Greece
    Show System Specs
    That sounds reasonable. At least for the type of plugins that currently exist, I think it would be enough.

    If need be it could be extended so plugins could "vote" for the minimum size each requires, then the image would be scaled to the max of the requests. But for now I think a fixed size is better.

    BTW: Can't the resize be done in the GPU (use the surface as a texture and draw it on a smaller secondary surface)? If that is possible, it would lower CPU usage even more.
     
  9. infinite.loop
    • Team MediaPortal

    infinite.loop Retired Team Member

    Joined:
    December 26, 2004
    Messages:
    16,163
    Likes Received:
    3,861
    Gender:
    Male
    Location:
    127.0.0.1
    Ratings:
    +4,154 / 7
    Home Country:
    Austria Austria
    Show System Specs
    Evenen though the proposed change sounds easy (in the first post at least, not so much later), i want to say that "the rules apply to everyone".
    hwharmann was also not allowed to add his minor change for the clock feature. And others to add simmilar small new features since we have a featurefreeze.

    Please lets focus on 1.1.0 (RC1/final) or MP2. :)

    :D
     
  10. tourettes
    • Team MediaPortal

    tourettes Retired Team Member

    Joined:
    January 7, 2005
    Messages:
    17,301
    Likes Received:
    4,595
    Ratings:
    +4,810 / 3
    Should be possible.
     
  11. tourettes
    • Team MediaPortal

    tourettes Retired Team Member

    Joined:
    January 7, 2005
    Messages:
    17,301
    Likes Received:
    4,595
    Ratings:
    +4,810 / 3
    I think the correct solution would be to force plugin to use a worked thread to do the analysis. So, to do it properly would require more changes than the proposed one -> voting for > 1.1.0 as target.
     
Loading...

Users Viewing Thread (Users: 0, Guests: 0)

  1. This site uses cookies to help personalise content, tailor your experience and to keep you logged in if you register.
    By continuing to use this site, you are consenting to our use of cookies.
    Dismiss Notice
  • About The Project

    The vision of the MediaPortal project is to create a free open source media centre application, which supports all advanced media centre functions, and is accessible to all Windows users.

    In reaching this goal we are working every day to make sure our software is one of the best.

             

  • Support MediaPortal!

    The team works very hard to make sure the community is running the best HTPC-software. We give away MediaPortal for free but hosting and software is not for us.

    Care to support our work with a few bucks? We'd really appreciate it!